Update compatible string with runtime detected chip revision
information, for example qcom,pm8941 will become qcom,pm8941-v1.0.
Signed-off-by: Ivan T. Ivanov <[email protected]>
---
.../devicetree/bindings/mfd/qcom,spmi-pmic.txt | 18 ++-
drivers/mfd/qcom-spmi-pmic.c | 142 +++++++++++++++++++++
2 files changed, 156 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt
index 7182b88..bbe7db8 100644
--- a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt
+++ b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt
@@ -15,10 +15,20 @@ each. A function can consume one or more of these fixed-size register regions.
Required properties:
- compatible: Should contain one of:
- "qcom,pm8941"
- "qcom,pm8841"
- "qcom,pma8084"
- or generalized "qcom,spmi-pmic".
+ qcom,pm8941,
+ qcom,pm8841,
+ qcom,pm8019,
+ qcom,pm8226,
+ qcom,pm8110,
+ qcom,pma8084,
+ qcom,pmi8962,
+ qcom,pmd9635,
+ qcom,pm8994,
+ qcom,pmi8994,
+ qcom,pm8916,
+ qcom,pm8004,
+ qcom,pm8909,
+ or generalized "qcom,spmi-pmic".
- reg: Specifies the SPMI USID slave address for this device.
For more information see:
Documentation/devicetree/bindings/spmi/spmi.txt
diff --git a/drivers/mfd/qcom-spmi-pmic.c b/drivers/mfd/qcom-spmi-pmic.c
index 4b8beb2..67446a4 100644
--- a/drivers/mfd/qcom-spmi-pmic.c
+++ b/drivers/mfd/qcom-spmi-pmic.c
@@ -13,10 +13,126 @@
#include <linux/kernel.h>
#include <linux/module.h>
+#include <linux/slab.h>
#include <linux/spmi.h>
#include <linux/regmap.h>
+#include <linux/of.h>
#include <linux/of_platform.h>
+#define PMIC_REV2 0x101
+#define PMIC_REV3 0x102
+#define PMIC_REV4 0x103
+#define PMIC_TYPE 0x104
+#define PMIC_SUBTYPE 0x105
+
+#define PMIC_TYPE_VALUE 0x51
+
+#define PM8941_SUBTYPE 0x01
+#define PM8841_SUBTYPE 0x02
+#define PM8019_SUBTYPE 0x03
+#define PM8226_SUBTYPE 0x04
+#define PM8110_SUBTYPE 0x05
+#define PMA8084_SUBTYPE 0x06
+#define PMI8962_SUBTYPE 0x07
+#define PMD9635_SUBTYPE 0x08
+#define PM8994_SUBTYPE 0x09
+#define PMI8994_SUBTYPE 0x0a
+#define PM8916_SUBTYPE 0x0b
+#define PM8004_SUBTYPE 0x0c
+#define PM8909_SUBTYPE 0x0d
+
+static int pmic_spmi_read_revid(struct regmap *map, char **name,
+ int *major, int *minor)
+{
+ unsigned int rev2, rev3, rev4, type, subtype;
+ int ret;
+
+ ret = regmap_read(map, PMIC_TYPE, &type);
+ if (ret < 0)
+ return ret;
+
+ if (type != PMIC_TYPE_VALUE)
+ return -EINVAL;
+
+ ret = regmap_read(map, PMIC_SUBTYPE, &subtype);
+ if (ret < 0)
+ return ret;
+
+ rev2 = regmap_read(map, PMIC_REV2, &rev2);
+ if (ret < 0)
+ return ret;
+
+ rev3 = regmap_read(map, PMIC_REV3, &rev3);
+ if (ret < 0)
+ return ret;
+
+ rev4 = regmap_read(map, PMIC_REV4, &rev4);
+ if (ret < 0)
+ return ret;
+
+ /*
+ * In early versions of PM8941 and PM8226, the major revision number
+ * started incrementing from 0 (eg 0 = v1.0, 1 = v2.0).
+ * Increment the major revision number here if the chip is an early
+ * version of PM8941 or PM8226.
+ */
+ if ((subtype == PM8941_SUBTYPE || subtype == PM8226_SUBTYPE) &&
+ rev4 < 0x02)
+ rev4++;
+
+ *major = rev4;
+ if (subtype == PM8110_SUBTYPE)
+ *minor = rev2;
+ else
+ *minor = rev3;
+
+ switch (subtype) {
+ case PM8941_SUBTYPE:
+ *name = "pm8941";
+ break;
+ case PM8841_SUBTYPE:
+ *name = "pm8841";
+ break;
+ case PM8019_SUBTYPE:
+ *name = "pm8019";
+ break;
+ case PM8226_SUBTYPE:
+ *name = "pm8226";
+ break;
+ case PM8110_SUBTYPE:
+ *name = "pm8110";
+ break;
+ case PMA8084_SUBTYPE:
+ *name = "pma8084";
+ break;
+ case PMI8962_SUBTYPE:
+ *name = "pmi8962";
+ break;
+ case PMD9635_SUBTYPE:
+ *name = "pmd8635";
+ break;
+ case PM8994_SUBTYPE:
+ *name = "pm8994";
+ break;
+ case PMI8994_SUBTYPE:
+ *name = "pmi8994";
+ break;
+ case PM8916_SUBTYPE:
+ *name = "pm8916";
+ break;
+ case PM8004_SUBTYPE:
+ *name = "pm8004";
+ break;
+ case PM8909_SUBTYPE:
+ *name = "pm8909";
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static const struct regmap_config spmi_regmap_config = {
.reg_bits = 16,
.val_bits = 8,
@@ -28,11 +144,27 @@ static int pmic_spmi_probe(struct spmi_device *sdev)
{
struct device_node *root = sdev->dev.of_node;
struct regmap *regmap;
+ struct property *prop;
+ int major, minor, ret;
+ char *name, compatible[32];
regmap = devm_regmap_init_spmi_ext(sdev, &spmi_regmap_config);
if (IS_ERR(regmap))
return PTR_ERR(regmap);
+ ret = pmic_spmi_read_revid(regmap, &name, &major, &minor);
+ if (!ret) {
+ snprintf(compatible, ARRAY_SIZE(compatible), "qcom,%s-v%d.%d",
+ name, major, minor);
+ prop = kzalloc(sizeof(*prop), GFP_KERNEL);
+ if (prop) {
+ prop->name = kstrdup("compatible", GFP_KERNEL);
+ prop->value = kstrdup(compatible, GFP_KERNEL);
+ prop->length = strlen(prop->value);
+ of_update_property(root, prop);
+ }
+ }
+
return of_platform_populate(root, NULL, NULL, &sdev->dev);
}
@@ -45,7 +177,17 @@ static const struct of_device_id pmic_spmi_id_table[] = {
{ .compatible = "qcom,spmi-pmic" },
{ .compatible = "qcom,pm8941" },
{ .compatible = "qcom,pm8841" },
+ { .compatible = "qcom,pm8019" },
+ { .compatible = "qcom,pm8226" },
+ { .compatible = "qcom,pm8110" },
{ .compatible = "qcom,pma8084" },
+ { .compatible = "qcom,pmi8962" },
+ { .compatible = "qcom,pmd9635" },
+ { .compatible = "qcom,pm8994" },
+ { .compatible = "qcom,pmi8994" },
+ { .compatible = "qcom,pm8916" },
+ { .compatible = "qcom,pm8004" },
+ { .compatible = "qcom,pm8909" },
{ }
};
MODULE_DEVICE_TABLE(of, pmic_spmi_id_table);
--
1.9.1
On Tue, Nov 4, 2014 at 11:33 AM, Ivan T. Ivanov <[email protected]> wrote:
> + ret = regmap_read(map, PMIC_SUBTYPE, &subtype);
> + if (ret < 0)
> + return ret;
> +
> + rev2 = regmap_read(map, PMIC_REV2, &rev2);
> + if (ret < 0)
> + return ret;
I think you meant:
ret = regmap_read(map, PMIC_REV2, &rev2);
> +
> + rev3 = regmap_read(map, PMIC_REV3, &rev3);
Likewise.
> + if (ret < 0)
> + return ret;
> +
> + rev4 = regmap_read(map, PMIC_REV4, &rev4);
Likewise.
> + if (ret < 0)
> + return ret;
Hi Ivan,
Thanks for the patch!
On 11/04/2014 03:33 PM, Ivan T. Ivanov wrote:
> Update compatible string with runtime detected chip revision
> information, for example qcom,pm8941 will become qcom,pm8941-v1.0.
>
> Signed-off-by: Ivan T. Ivanov <[email protected]>
> ---
> .../devicetree/bindings/mfd/qcom,spmi-pmic.txt | 18 ++-
> drivers/mfd/qcom-spmi-pmic.c | 142 +++++++++++++++++++++
> 2 files changed, 156 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt
> index 7182b88..bbe7db8 100644
> --- a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt
> +++ b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt
> @@ -15,10 +15,20 @@ each. A function can consume one or more of these fixed-size register regions.
>
> Required properties:
> - compatible: Should contain one of:
> - "qcom,pm8941"
> - "qcom,pm8841"
> - "qcom,pma8084"
> - or generalized "qcom,spmi-pmic".
> + qcom,pm8941,
> + qcom,pm8841,
> + qcom,pm8019,
> + qcom,pm8226,
> + qcom,pm8110,
> + qcom,pma8084,
> + qcom,pmi8962,
> + qcom,pmd9635,
> + qcom,pm8994,
> + qcom,pmi8994,
> + qcom,pm8916,
> + qcom,pm8004,
> + qcom,pm8909,
> + or generalized "qcom,spmi-pmic".
> - reg: Specifies the SPMI USID slave address for this device.
> For more information see:
> Documentation/devicetree/bindings/spmi/spmi.txt
> diff --git a/drivers/mfd/qcom-spmi-pmic.c b/drivers/mfd/qcom-spmi-pmic.c
> index 4b8beb2..67446a4 100644
> --- a/drivers/mfd/qcom-spmi-pmic.c
> +++ b/drivers/mfd/qcom-spmi-pmic.c
> @@ -13,10 +13,126 @@
>
> #include <linux/kernel.h>
> #include <linux/module.h>
> +#include <linux/slab.h>
> #include <linux/spmi.h>
> #include <linux/regmap.h>
> +#include <linux/of.h>
> #include <linux/of_platform.h>
>
> +#define PMIC_REV2 0x101
> +#define PMIC_REV3 0x102
> +#define PMIC_REV4 0x103
> +#define PMIC_TYPE 0x104
> +#define PMIC_SUBTYPE 0x105
> +
> +#define PMIC_TYPE_VALUE 0x51
> +
> +#define PM8941_SUBTYPE 0x01
> +#define PM8841_SUBTYPE 0x02
> +#define PM8019_SUBTYPE 0x03
> +#define PM8226_SUBTYPE 0x04
> +#define PM8110_SUBTYPE 0x05
> +#define PMA8084_SUBTYPE 0x06
> +#define PMI8962_SUBTYPE 0x07
> +#define PMD9635_SUBTYPE 0x08
> +#define PM8994_SUBTYPE 0x09
> +#define PMI8994_SUBTYPE 0x0a
> +#define PM8916_SUBTYPE 0x0b
> +#define PM8004_SUBTYPE 0x0c
> +#define PM8909_SUBTYPE 0x0d
> +
> +static int pmic_spmi_read_revid(struct regmap *map, char **name,
> + int *major, int *minor)
> +{
> + unsigned int rev2, rev3, rev4, type, subtype;
> + int ret;
> +
> + ret = regmap_read(map, PMIC_TYPE, &type);
> + if (ret < 0)
> + return ret;
> +
> + if (type != PMIC_TYPE_VALUE)
> + return -EINVAL;
> +
> + ret = regmap_read(map, PMIC_SUBTYPE, &subtype);
> + if (ret < 0)
> + return ret;
> +
> + rev2 = regmap_read(map, PMIC_REV2, &rev2);
> + if (ret < 0)
> + return ret;
> +
> + rev3 = regmap_read(map, PMIC_REV3, &rev3);
> + if (ret < 0)
> + return ret;
> +
> + rev4 = regmap_read(map, PMIC_REV4, &rev4);
> + if (ret < 0)
> + return ret;
> +
> + /*
> + * In early versions of PM8941 and PM8226, the major revision number
> + * started incrementing from 0 (eg 0 = v1.0, 1 = v2.0).
> + * Increment the major revision number here if the chip is an early
> + * version of PM8941 or PM8226.
> + */
> + if ((subtype == PM8941_SUBTYPE || subtype == PM8226_SUBTYPE) &&
> + rev4 < 0x02)
> + rev4++;
> +
> + *major = rev4;
> + if (subtype == PM8110_SUBTYPE)
> + *minor = rev2;
> + else
> + *minor = rev3;
> +
> + switch (subtype) {
> + case PM8941_SUBTYPE:
> + *name = "pm8941";
> + break;
The XXX_SUBTYPE seems are continuous why not make it an const array and
get the name by index in this array?
> + case PM8841_SUBTYPE:
> + *name = "pm8841";
> + break;
> + case PM8019_SUBTYPE:
> + *name = "pm8019";
> + break;
> + case PM8226_SUBTYPE:
> + *name = "pm8226";
> + break;
> + case PM8110_SUBTYPE:
> + *name = "pm8110";
> + break;
> + case PMA8084_SUBTYPE:
> + *name = "pma8084";
> + break;
> + case PMI8962_SUBTYPE:
> + *name = "pmi8962";
> + break;
> + case PMD9635_SUBTYPE:
> + *name = "pmd8635";
> + break;
> + case PM8994_SUBTYPE:
> + *name = "pm8994";
> + break;
> + case PMI8994_SUBTYPE:
> + *name = "pmi8994";
> + break;
> + case PM8916_SUBTYPE:
> + *name = "pm8916";
> + break;
> + case PM8004_SUBTYPE:
> + *name = "pm8004";
> + break;
> + case PM8909_SUBTYPE:
> + *name = "pm8909";
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> static const struct regmap_config spmi_regmap_config = {
> .reg_bits = 16,
> .val_bits = 8,
> @@ -28,11 +144,27 @@ static int pmic_spmi_probe(struct spmi_device *sdev)
> {
> struct device_node *root = sdev->dev.of_node;
> struct regmap *regmap;
> + struct property *prop;
> + int major, minor, ret;
> + char *name, compatible[32];
>
> regmap = devm_regmap_init_spmi_ext(sdev, &spmi_regmap_config);
> if (IS_ERR(regmap))
> return PTR_ERR(regmap);
>
> + ret = pmic_spmi_read_revid(regmap, &name, &major, &minor);
> + if (!ret) {
Are you sure that we want to continue if we can't read the revision id
and therefore will not be able to construct properly the compatible
property?
> + snprintf(compatible, ARRAY_SIZE(compatible), "qcom,%s-v%d.%d",
> + name, major, minor);
> + prop = kzalloc(sizeof(*prop), GFP_KERNEL);
> + if (prop) {
> + prop->name = kstrdup("compatible", GFP_KERNEL);
> + prop->value = kstrdup(compatible, GFP_KERNEL);
> + prop->length = strlen(prop->value);
> + of_update_property(root, prop);
of_update_property can fail, check the returned value.
<snip>
--
regards,
Stan
On Tue, 2014-11-04 at 12:56 -0200, Fabio Estevam wrote:
> On Tue, Nov 4, 2014 at 11:33 AM, Ivan T. Ivanov <[email protected]> wrote:
>
> > + ret = regmap_read(map, PMIC_SUBTYPE, &subtype);
> > + if (ret < 0)
> > + return ret;
> > +
> > + rev2 = regmap_read(map, PMIC_REV2, &rev2);
> > + if (ret < 0)
> > + return ret;
>
> I think you meant:
> ret = regmap_read(map, PMIC_REV2, &rev2);
>
> > +
> > + rev3 = regmap_read(map, PMIC_REV3, &rev3);
>
> Likewise.
>
> > + if (ret < 0)
> > + return ret;
> > +
> > + rev4 = regmap_read(map, PMIC_REV4, &rev4);
>
> Likewise.
>
> >
True. Will fix.
Thank you.
Ivan
On Tue, 2014-11-04 at 17:06 +0200, Stanimir Varbanov wrote:
> Hi Ivan,
>
>
+
> > + switch (subtype) {
> > + case PM8941_SUBTYPE:
> > + *name = "pm8941";
> > + break;
>
> The XXX_SUBTYPE seems are continuous why not make it an const array and
> get the name by index in this array?
>
Yep, it _seems_ to be continuous. But, yes. probably using array will
more compact way to represent this.
<snip>
> > @@ -28,11 +144,27 @@ static int pmic_spmi_probe(struct spmi_device *sdev)
> > {
> > struct device_node *root = sdev->dev.of_node;
> > struct regmap *regmap;
> > + struct property *prop;
> > + int major, minor, ret;
> > + char *name, compatible[32];
> >
> > regmap = devm_regmap_init_spmi_ext(sdev, &spmi_regmap_config);
> > if (IS_ERR(regmap))
> > return PTR_ERR(regmap);
> >
> > + ret = pmic_spmi_read_revid(regmap, &name, &major, &minor);
> > + if (!ret) {
>
> Are you sure that we want to continue if we can't read the revision id
> and therefore will not be able to construct properly the compatible
> property?
>
Yes. Driver is working fine even without exact chip version
appended to compatible string.
> > + snprintf(compatible, ARRAY_SIZE(compatible), "qcom,%s-v%d.%d",
> > + name, major, minor);
> > + prop = kzalloc(sizeof(*prop), GFP_KERNEL);
> > + if (prop) {
> > + prop->name = kstrdup("compatible", GFP_KERNEL);
> > + prop->value = kstrdup(compatible, GFP_KERNEL);
> > + prop->length = strlen(prop->value);
> > + of_update_property(root, prop);
>
> of_update_property can fail, check the returned value.
Same thing as above, but probably allocated memory at least can be freed.
Thanks Ivan.
>
> <snip>
>
>
On 11/04/2014 05:22 PM, Ivan T. Ivanov wrote:
>
> On Tue, 2014-11-04 at 17:06 +0200, Stanimir Varbanov wrote:
>> Hi Ivan,
>>
>>
> +
>>> + switch (subtype) {
>>> + case PM8941_SUBTYPE:
>>> + *name = "pm8941";
>>> + break;
>>
>> The XXX_SUBTYPE seems are continuous why not make it an const array and
>> get the name by index in this array?
>>
>
> Yep, it _seems_ to be continuous. But, yes. probably using array will
> more compact way to represent this.
>
> <snip>
>
>>> @@ -28,11 +144,27 @@ static int pmic_spmi_probe(struct spmi_device *sdev)
>>> {
>>> struct device_node *root = sdev->dev.of_node;
>>> struct regmap *regmap;
>>> + struct property *prop;
>>> + int major, minor, ret;
>>> + char *name, compatible[32];
>>>
>>> regmap = devm_regmap_init_spmi_ext(sdev, &spmi_regmap_config);
>>> if (IS_ERR(regmap))
>>> return PTR_ERR(regmap);
>>>
>>> + ret = pmic_spmi_read_revid(regmap, &name, &major, &minor);
>>> + if (!ret) {
>>
>> Are you sure that we want to continue if we can't read the revision id
>> and therefore will not be able to construct properly the compatible
>> property?
>>
>
> Yes. Driver is working fine even without exact chip version
> appended to compatible string.
>
>>> + snprintf(compatible, ARRAY_SIZE(compatible), "qcom,%s-v%d.%d",
>>> + name, major, minor);
>>> + prop = kzalloc(sizeof(*prop), GFP_KERNEL);
>>> + if (prop) {
>>> + prop->name = kstrdup("compatible", GFP_KERNEL);
>>> + prop->value = kstrdup(compatible, GFP_KERNEL);
>>> + prop->length = strlen(prop->value);
>>> + of_update_property(root, prop);
>>
>> of_update_property can fail, check the returned value.
>
> Same thing as above, but probably allocated memory at least can be freed.
might be better idea to use devm_kzalloc and devm_kstrdup?
--
regards,
Stan
On Tue, 2014-11-04 at 17:26 +0200, Stanimir Varbanov wrote:
> On 11/04/2014 05:22 PM, Ivan T. Ivanov wrote:
> > On Tue, 2014-11-04 at 17:06 +0200, Stanimir Varbanov wrote:
<snip>
> > > > + snprintf(compatible, ARRAY_SIZE(compatible), "qcom,%s-v%d.%d",
> > > > + name, major, minor);
> > > > + prop = kzalloc(sizeof(*prop), GFP_KERNEL);
> > > > + if (prop) {
> > > > + prop->name = kstrdup("compatible", GFP_KERNEL);
> > > > + prop->value = kstrdup(compatible, GFP_KERNEL);
> > > > + prop->length = strlen(prop->value);
> > > > + of_update_property(root, prop);
> > >
> > > of_update_property can fail, check the returned value.
> >
> > Same thing as above, but probably allocated memory at least can be freed.
>
> might be better idea to use devm_kzalloc and devm_kstrdup?
>
compatible property is attached to device not to driver, so
memory should be there even after driver is unloaded, I think.
Regards,
Ivan
Hi,
Am 04.11.2014 um 14:33 schrieb Ivan T. Ivanov:
> Update compatible string with runtime detected chip revision
> information, for example qcom,pm8941 will become qcom,pm8941-v1.0.
That's not what the patch does though?
>
> Signed-off-by: Ivan T. Ivanov <[email protected]>
> ---
> .../devicetree/bindings/mfd/qcom,spmi-pmic.txt | 18 ++-
> drivers/mfd/qcom-spmi-pmic.c | 142 +++++++++++++++++++++
> 2 files changed, 156 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt
> index 7182b88..bbe7db8 100644
> --- a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt
> +++ b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt
> @@ -15,10 +15,20 @@ each. A function can consume one or more of these fixed-size register regions.
>
> Required properties:
> - compatible: Should contain one of:
> - "qcom,pm8941"
> - "qcom,pm8841"
> - "qcom,pma8084"
> - or generalized "qcom,spmi-pmic".
> + qcom,pm8941,
> + qcom,pm8841,
> + qcom,pm8019,
> + qcom,pm8226,
> + qcom,pm8110,
> + qcom,pma8084,
> + qcom,pmi8962,
> + qcom,pmd9635,
> + qcom,pm8994,
> + qcom,pmi8994,
> + qcom,pm8916,
> + qcom,pm8004,
> + qcom,pm8909,
Please either keep the strings consistently quoted, or drop the trailing
comma to avoid any confusion of where it terminates.
> + or generalized "qcom,spmi-pmic".
> - reg: Specifies the SPMI USID slave address for this device.
> For more information see:
> Documentation/devicetree/bindings/spmi/spmi.txt
Regards,
Andreas
[...]
> @@ -45,7 +177,17 @@ static const struct of_device_id pmic_spmi_id_table[] = {
> { .compatible = "qcom,spmi-pmic" },
> { .compatible = "qcom,pm8941" },
> { .compatible = "qcom,pm8841" },
> + { .compatible = "qcom,pm8019" },
> + { .compatible = "qcom,pm8226" },
> + { .compatible = "qcom,pm8110" },
> { .compatible = "qcom,pma8084" },
> + { .compatible = "qcom,pmi8962" },
> + { .compatible = "qcom,pmd9635" },
> + { .compatible = "qcom,pm8994" },
> + { .compatible = "qcom,pmi8994" },
> + { .compatible = "qcom,pm8916" },
> + { .compatible = "qcom,pm8004" },
> + { .compatible = "qcom,pm8909" },
> { }
> };
> MODULE_DEVICE_TABLE(of, pmic_spmi_id_table);
--
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imend?rffer; HRB 21284 AG N?rnberg
On Wed, 2014-11-05 at 13:49 +0100, Andreas Färber wrote:
> Hi,
>
> Am 04.11.2014 um 14:33 schrieb Ivan T. Ivanov:
> > Update compatible string with runtime detected chip revision
> > information, for example qcom,pm8941 will become qcom,pm8941-v1.0.
>
> That's not what the patch does though?
Patch add support for more chips and make compatible property more specific.
Will correct description.
>
> > Signed-off-by: Ivan T. Ivanov <[email protected]>
> > ---
> > .../devicetree/bindings/mfd/qcom,spmi-pmic.txt | 18 ++-
> > drivers/mfd/qcom-spmi-pmic.c | 142 +++++++++++++++++++++
> > 2 files changed, 156 insertions(+), 4 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt
> > b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt
> > index 7182b88..bbe7db8 100644
> > --- a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt
> > +++ b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.txt
> > @@ -15,10 +15,20 @@ each. A function can consume one or more of these fixed-size register
> > regions.
> >
> > Required properties:
> > - compatible: Should contain one of:
> > - "qcom,pm8941"
> > - "qcom,pm8841"
> > - "qcom,pma8084"
> > - or generalized "qcom,spmi-pmic".
> > + qcom,pm8941,
> > + qcom,pm8841,
> > + qcom,pm8019,
> > + qcom,pm8226,
> > + qcom,pm8110,
> > + qcom,pma8084,
> > + qcom,pmi8962,
> > + qcom,pmd9635,
> > + qcom,pm8994,
> > + qcom,pmi8994,
> > + qcom,pm8916,
> > + qcom,pm8004,
> > + qcom,pm8909,
>
> Please either keep the strings consistently quoted, or drop the trailing
> comma to avoid any confusion of where it terminates.
Sure, will fix it.
Thank you,
Ivan
On Tue, Nov 4, 2014 at 5:33 AM, Ivan T. Ivanov <[email protected]> wrote:
[..]
> @@ -28,11 +144,27 @@ static int pmic_spmi_probe(struct spmi_device *sdev)
> {
> struct device_node *root = sdev->dev.of_node;
> struct regmap *regmap;
> + struct property *prop;
> + int major, minor, ret;
> + char *name, compatible[32];
>
> regmap = devm_regmap_init_spmi_ext(sdev, &spmi_regmap_config);
> if (IS_ERR(regmap))
> return PTR_ERR(regmap);
>
> + ret = pmic_spmi_read_revid(regmap, &name, &major, &minor);
> + if (!ret) {
> + snprintf(compatible, ARRAY_SIZE(compatible), "qcom,%s-v%d.%d",
> + name, major, minor);
> + prop = kzalloc(sizeof(*prop), GFP_KERNEL);
> + if (prop) {
> + prop->name = kstrdup("compatible", GFP_KERNEL);
> + prop->value = kstrdup(compatible, GFP_KERNEL);
> + prop->length = strlen(prop->value);
> + of_update_property(root, prop);
> + }
> + }
> +
Why would you do this?
What benefit does it give to patch the of_node to have a more specific
compatible?
It is no longer matching any compatible defined in the kernel and is
anyone actually looking at this?
Reading out the revid information and providing that in some way to
the children could be beneficial, except for qpnp already giving you
this version information per block.
[..]
> @@ -45,7 +177,17 @@ static const struct of_device_id pmic_spmi_id_table[] = {
> { .compatible = "qcom,spmi-pmic" },
> { .compatible = "qcom,pm8941" },
> { .compatible = "qcom,pm8841" },
> + { .compatible = "qcom,pm8019" },
> + { .compatible = "qcom,pm8226" },
> + { .compatible = "qcom,pm8110" },
> { .compatible = "qcom,pma8084" },
> + { .compatible = "qcom,pmi8962" },
> + { .compatible = "qcom,pmd9635" },
> + { .compatible = "qcom,pm8994" },
> + { .compatible = "qcom,pmi8994" },
> + { .compatible = "qcom,pm8916" },
> + { .compatible = "qcom,pm8004" },
> + { .compatible = "qcom,pm8909" },
> { }
This part is good, please send this out on it's own.
Regards,
Bjorn
On Wed, 2014-11-05 at 10:11 -0800, Bjorn Andersson wrote:
> On Tue, Nov 4, 2014 at 5:33 AM, Ivan T. Ivanov <[email protected]>
> wrote:
> [..]
> > @@ -28,11 +144,27 @@ static int pmic_spmi_probe(struct spmi_device
> > *sdev)
> > {
> > struct device_node *root = sdev->dev.of_node;
> > struct regmap *regmap;
> > + struct property *prop;
> > + int major, minor, ret;
> > + char *name, compatible[32];
> >
> > regmap = devm_regmap_init_spmi_ext(sdev,
> > &spmi_regmap_config);
> > if (IS_ERR(regmap))
> > return PTR_ERR(regmap);
> >
> > + ret = pmic_spmi_read_revid(regmap, &name, &major, &minor);
> > + if (!ret) {
> > + snprintf(compatible, ARRAY_SIZE(compatible),
> > "qcom,%s-v%d.%d",
> > + name, major, minor);
> > + prop = kzalloc(sizeof(*prop), GFP_KERNEL);
> > + if (prop) {
> > + prop->name = kstrdup("compatible",
> > GFP_KERNEL);
> > + prop->value = kstrdup(compatible,
> > GFP_KERNEL);
> > + prop->length = strlen(prop->value);
> > + of_update_property(root, prop);
> > + }
> > + }
> > +
>
> Why would you do this?
> What benefit does it give to patch the of_node to have a more
> specific
> compatible?
Some of the child device drivers have to know PMIC chip revision.
Regards,
Ivan
On Wed, Nov 5, 2014 at 10:31 AM, Ivan T. Ivanov <[email protected]> wrote:
>
> On Wed, 2014-11-05 at 10:11 -0800, Bjorn Andersson wrote:
>> On Tue, Nov 4, 2014 at 5:33 AM, Ivan T. Ivanov <[email protected]>
>> wrote:
>> [..]
>> > @@ -28,11 +144,27 @@ static int pmic_spmi_probe(struct spmi_device
>> > *sdev)
>> > {
>> > struct device_node *root = sdev->dev.of_node;
>> > struct regmap *regmap;
>> > + struct property *prop;
>> > + int major, minor, ret;
>> > + char *name, compatible[32];
>> >
>> > regmap = devm_regmap_init_spmi_ext(sdev,
>> > &spmi_regmap_config);
>> > if (IS_ERR(regmap))
>> > return PTR_ERR(regmap);
>> >
>> > + ret = pmic_spmi_read_revid(regmap, &name, &major, &minor);
>> > + if (!ret) {
>> > + snprintf(compatible, ARRAY_SIZE(compatible),
>> > "qcom,%s-v%d.%d",
>> > + name, major, minor);
>> > + prop = kzalloc(sizeof(*prop), GFP_KERNEL);
>> > + if (prop) {
>> > + prop->name = kstrdup("compatible",
>> > GFP_KERNEL);
>> > + prop->value = kstrdup(compatible,
>> > GFP_KERNEL);
>> > + prop->length = strlen(prop->value);
>> > + of_update_property(root, prop);
>> > + }
>> > + }
>> > +
>>
>> Why would you do this?
>> What benefit does it give to patch the of_node to have a more
>> specific
>> compatible?
>
> Some of the child device drivers have to know PMIC chip revision.
>
So your plan is to have a strstr(parent->compatible, "-v2") there?
Could you be a little bit more elaborate on what you're trying to do
and which child devices that might be?
Regards,
Bjorn
On Wed, 2014-11-05 at 17:36 -0800, Bjorn Andersson wrote:
> On Wed, Nov 5, 2014 at 10:31 AM, Ivan T. Ivanov <[email protected]> wrote:
> > On Wed, 2014-11-05 at 10:11 -0800, Bjorn Andersson wrote:
> > > On Tue, Nov 4, 2014 at 5:33 AM, Ivan T. Ivanov <[email protected]>
> > > wrote:
> > > [..]
> > > > @@ -28,11 +144,27 @@ static int pmic_spmi_probe(struct spmi_device
> > > > *sdev)
> > > > {
> > > > struct device_node *root = sdev->dev.of_node;
> > > > struct regmap *regmap;
> > > > + struct property *prop;
> > > > + int major, minor, ret;
> > > > + char *name, compatible[32];
> > > >
> > > > regmap = devm_regmap_init_spmi_ext(sdev,
> > > > &spmi_regmap_config);
> > > > if (IS_ERR(regmap))
> > > > return PTR_ERR(regmap);
> > > >
> > > > + ret = pmic_spmi_read_revid(regmap, &name, &major, &minor);
> > > > + if (!ret) {
> > > > + snprintf(compatible, ARRAY_SIZE(compatible),
> > > > "qcom,%s-v%d.%d",
> > > > + name, major, minor);
> > > > + prop = kzalloc(sizeof(*prop), GFP_KERNEL);
> > > > + if (prop) {
> > > > + prop->name = kstrdup("compatible",
> > > > GFP_KERNEL);
> > > > + prop->value = kstrdup(compatible,
> > > > GFP_KERNEL);
> > > > + prop->length = strlen(prop->value);
> > > > + of_update_property(root, prop);
> > > > + }
> > > > + }
> > > > +
> > >
> > > Why would you do this?
> > > What benefit does it give to patch the of_node to have a more
> > > specific
> > > compatible?
> >
> > Some of the child device drivers have to know PMIC chip revision.
> >
>
> So your plan is to have a strstr(parent->compatible, "-v2") there?
Actually also PMIC subtype (pm8841, pm8226...) is also required, so
the plan is to have something like this:
{
static const struct of_device_id pmic_match_table[] = {
{ .compatible = "qcom,pm8941-v1.0" },
{ .compatible = "qcom,pm8841-v0.0" },
{ }
};
const struct of_device_id *match;
match = of_match_device(pmic_match_table, pdev->dev.parent);
if (match) {
dev_info(&pdev->dev, "%s chip detected\n", match->compatible);
}
}
>
> Could you be a little bit more elaborate on what you're trying to do
> and which child devices that might be?
For example ADC drivers are required temperature compensation based
on PMIC variant and chip manufacturer.
This patch have one issue, at least :-). Using of_update_property will prevent
driver to be build as module. which, I think, is coming from the fact the
on first load it will modify device compatible property and will be impossible
driver to match device id again. Still thinking how to overcome this.
Regards,
Ivan
On Wed, Nov 5, 2014 at 11:54 PM, Ivan T. Ivanov <[email protected]> wrote:
>
> On Wed, 2014-11-05 at 17:36 -0800, Bjorn Andersson wrote:
>> On Wed, Nov 5, 2014 at 10:31 AM, Ivan T. Ivanov <[email protected]> wrote:
[..]
>> > Some of the child device drivers have to know PMIC chip revision.
>> >
>>
>> So your plan is to have a strstr(parent->compatible, "-v2") there?
>
> Actually also PMIC subtype (pm8841, pm8226...) is also required, so
> the plan is to have something like this:
>
> {
> static const struct of_device_id pmic_match_table[] = {
> { .compatible = "qcom,pm8941-v1.0" },
> { .compatible = "qcom,pm8841-v0.0" },
> { }
>
> };
>
> const struct of_device_id *match;
>
> match = of_match_device(pmic_match_table, pdev->dev.parent);
> if (match) {
> dev_info(&pdev->dev, "%s chip detected\n", match->compatible);
> }
> }
>
To me this is a hack, you should not alter the devicetree to make it
"better express the hardware". Either you know these things from boot
and they go in device tree, or you can probe them and they should not
go in device tree.
If you really need these values you should expose them through some api.
>>
>> Could you be a little bit more elaborate on what you're trying to do
>> and which child devices that might be?
>
> For example ADC drivers are required temperature compensation based
> on PMIC variant and chip manufacturer.
>
I see, is that compensation of any practical value? Or is the
compensation of academic proportions?
Regards,
Bjorn
On Thu, 2014-11-06 at 08:55 -0800, Bjorn Andersson wrote:
> On Wed, Nov 5, 2014 at 11:54 PM, Ivan T. Ivanov <[email protected]> wrote:
> > On Wed, 2014-11-05 at 17:36 -0800, Bjorn Andersson wrote:
> > > On Wed, Nov 5, 2014 at 10:31 AM, Ivan T. Ivanov <[email protected]> wrote:
> [..]
> > > > Some of the child device drivers have to know PMIC chip revision.
> > > >
> > >
> > > So your plan is to have a strstr(parent->compatible, "-v2") there?
> >
> > Actually also PMIC subtype (pm8841, pm8226...) is also required, so
> > the plan is to have something like this:
> >
> > {
> > static const struct of_device_id pmic_match_table[] = {
> > { .compatible = "qcom,pm8941-v1.0" },
> > { .compatible = "qcom,pm8841-v0.0" },
> > { }
> >
> > };
> >
> > const struct of_device_id *match;
> >
> > match = of_match_device(pmic_match_table, pdev->dev.parent);
> > if (match) {
> > dev_info(&pdev->dev, "%s chip detected\n", match->compatible);
> > }
> > }
> >
>
> To me this is a hack, you should not alter the devicetree to make it
> "better express the hardware". Either you know these things from boot
> and they go in device tree, or you can probe them and they should not
> go in device tree.
>
> If you really need these values you should expose them through some api.
I would like to avoid compile time dependency between these drivers.
There are several precedents of using of_update_property() for enhancing
compatible property already.
>
> > > Could you be a little bit more elaborate on what you're trying to do
> > > and which child devices that might be?
> >
> > For example ADC drivers are required temperature compensation based
> > on PMIC variant and chip manufacturer.
> >
>
> I see, is that compensation of any practical value? Or is the
> compensation of academic proportions?
It depends of what you mean by academic :-). Attached file have test
application which dump difference between non compensated and compensated
values for different temperature, manufacture and input value.
Output format of the program is:
Column 1: manufacturer GF=0, SMIC=1, TSMC=2
Column 2: chip revision
Column 3: die temperature in mili deg Celsius
Column 4: input for compensation in micro Volts
Column 5: compensated result in micro Volts
Column 6: difference in micro Volts
Regards,
Ivan
On Fri, 2014-11-07 at 17:33 +0200, Ivan T. Ivanov wrote:
> On Thu, 2014-11-06 at 08:55 -0800, Bjorn Andersson wrote:
> > On Wed, Nov 5, 2014 at 11:54 PM, Ivan T. Ivanov <[email protected]> wrote:
> > > On Wed, 2014-11-05 at 17:36 -0800, Bjorn Andersson wrote:
> > > > On Wed, Nov 5, 2014 at 10:31 AM, Ivan T. Ivanov <[email protected]> wrote:
> > [..]
> > > > > Some of the child device drivers have to know PMIC chip revision.
> > > > >
> > > >
> > > > So your plan is to have a strstr(parent->compatible, "-v2") there?
> > >
> > > Actually also PMIC subtype (pm8841, pm8226...) is also required, so
> > > the plan is to have something like this:
> > >
> > > {
> > > static const struct of_device_id pmic_match_table[] = {
> > > { .compatible = "qcom,pm8941-v1.0" },
> > > { .compatible = "qcom,pm8841-v0.0" },
> > > { }
> > >
> > > };
> > >
> > > const struct of_device_id *match;
> > >
> > > match = of_match_device(pmic_match_table, pdev->dev.parent);
> > > if (match) {
> > > dev_info(&pdev->dev, "%s chip detected\n", match->compatible);
> > > }
> > > }
> > >
> >
> > To me this is a hack, you should not alter the devicetree to make it
> > "better express the hardware". Either you know these things from boot
> > and they go in device tree, or you can probe them and they should not
> > go in device tree.
> >
> > If you really need these values you should expose them through some api.
>
> I would like to avoid compile time dependency between these drivers.
> There are several precedents of using of_update_property() for enhancing
> compatible property already.
>
> > > > Could you be a little bit more elaborate on what you're trying to do
> > > > and which child devices that might be?
> > >
> > > For example ADC drivers are required temperature compensation based
> > > on PMIC variant and chip manufacturer.
> > >
> >
> > I see, is that compensation of any practical value? Or is the
> > compensation of academic proportions?
>
> It depends of what you mean by academic :-). Attached file have test
> application which dump difference between non compensated and compensated
> values for different temperature, manufacture and input value.
>
> Output format of the program is:
> Column 1: manufacturer GF=0, SMIC=1, TSMC=2
> Column 2: chip revision
> Column 3: die temperature in mili deg Celsius
> Column 4: input for compensation in micro Volts
> Column 5: compensated result in micro Volts
> Column 6: difference in micro Volts
Forgot to add. PMIC subtype and version are used also in charger and BMS
drivers to workaround hardware issues.
Ivan
On 11/6/2014 12:54 AM, Ivan T. Ivanov wrote:
> On Wed, 2014-11-05 at 17:36 -0800, Bjorn Andersson wrote:
>> On Wed, Nov 5, 2014 at 10:31 AM, Ivan T. Ivanov <[email protected]> wrote:
>>> On Wed, 2014-11-05 at 10:11 -0800, Bjorn Andersson wrote:
>>>> On Tue, Nov 4, 2014 at 5:33 AM, Ivan T. Ivanov <[email protected]>
>>>> wrote:
>>>> [..]
>>>>> @@ -28,11 +144,27 @@ static int pmic_spmi_probe(struct spmi_device
>>>>> *sdev)
>>>>> {
>>>>> struct device_node *root = sdev->dev.of_node;
>>>>> struct regmap *regmap;
>>>>> + struct property *prop;
>>>>> + int major, minor, ret;
>>>>> + char *name, compatible[32];
>>>>>
>>>>> regmap = devm_regmap_init_spmi_ext(sdev,
>>>>> &spmi_regmap_config);
Hi Ivan, I have a general question about this driver/layer.
Since the driver is using regmap, why does it need to be
qcom-*spmi*-pmic ? could we drop the spmi part?
regmap's point is abstraction of the bus technology, and indeed some
PMICs use i2c.
>>>>> if (IS_ERR(regmap))
>>>>> return PTR_ERR(regmap);
>>>>>
>>>>> + ret = pmic_spmi_read_revid(regmap, &name, &major, &minor);
>>>>> + if (!ret) {
>>>>> + snprintf(compatible, ARRAY_SIZE(compatible),
>>>>> "qcom,%s-v%d.%d",
>>>>> + name, major, minor);
>>>>> + prop = kzalloc(sizeof(*prop), GFP_KERNEL);
>>>>> + if (prop) {
>>>>> + prop->name = kstrdup("compatible",
>>>>> GFP_KERNEL);
>>>>> + prop->value = kstrdup(compatible,
>>>>> GFP_KERNEL);
>>>>> + prop->length = strlen(prop->value);
>>>>> + of_update_property(root, prop);
>>>>> + }
>>>>> + }
>>>>> +
>>>> Why would you do this?
>>>> What benefit does it give to patch the of_node to have a more
>>>> specific
>>>> compatible?
>>> Some of the child device drivers have to know PMIC chip revision.
>>>
>> So your plan is to have a strstr(parent->compatible, "-v2") there?
> Actually also PMIC subtype (pm8841, pm8226...) is also required, so
> the plan is to have something like this:
>
> {
> static const struct of_device_id pmic_match_table[] = {
> { .compatible = "qcom,pm8941-v1.0" },
> { .compatible = "qcom,pm8841-v0.0" },
> { }
>
> };
>
> const struct of_device_id *match;
>
> match = of_match_device(pmic_match_table, pdev->dev.parent);
> if (match) {
> dev_info(&pdev->dev, "%s chip detected\n", match->compatible);
> }
> }
>
>> Could you be a little bit more elaborate on what you're trying to do
>> and which child devices that might be?
> For example ADC drivers are required temperature compensation based
> on PMIC variant and chip manufacturer.
>
> This patch have one issue, at least :-). Using of_update_property will prevent
> driver to be build as module. which, I think, is coming from the fact the
> on first load it will modify device compatible property and will be impossible
> driver to match device id again. Still thinking how to overcome this.
>
> Regards,
> Ivan
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Hi Gilad,
On Fri, 2014-11-07 at 17:08 -0700, Gilad Avidov wrote:
> On 11/6/2014 12:54 AM, Ivan T. Ivanov wrote:
> > On Wed, 2014-11-05 at 17:36 -0800, Bjorn Andersson wrote:
> > > On Wed, Nov 5, 2014 at 10:31 AM, Ivan T. Ivanov <[email protected]> wrote:
> > > > On Wed, 2014-11-05 at 10:11 -0800, Bjorn Andersson wrote:
> > > > > On Tue, Nov 4, 2014 at 5:33 AM, Ivan T. Ivanov <[email protected]>
> > > > > wrote:
> > > > > [..]
> > > > > > @@ -28,11 +144,27 @@ static int pmic_spmi_probe(struct spmi_device
> > > > > > *sdev)
> > > > > > {
> > > > > > struct device_node *root = sdev->dev.of_node;
> > > > > > struct regmap *regmap;
> > > > > > + struct property *prop;
> > > > > > + int major, minor, ret;
> > > > > > + char *name, compatible[32];
> > > > > >
> > > > > > regmap = devm_regmap_init_spmi_ext(sdev,
> > > > > > &spmi_regmap_config);
> Hi Ivan, I have a general question about this driver/layer.
> Since the driver is using regmap, why does it need to be
> qcom-*spmi*-pmic ? could we drop the spmi part?
> regmap's point is abstraction of the bus technology, and indeed some
> PMICs use i2c.
This is driver for SPMI device, so no. The child device/drivers are
different question, but I don't want to start that discussion again.
Regards,
Ivan
On Fri, Nov 07, 2014 at 04:40:52PM +0100, Ivan T. Ivanov wrote:
> On Fri, 2014-11-07 at 17:33 +0200, Ivan T. Ivanov wrote:
> > On Thu, 2014-11-06 at 08:55 -0800, Bjorn Andersson wrote:
> > > On Wed, Nov 5, 2014 at 11:54 PM, Ivan T. Ivanov <[email protected]> wrote:
> > > > On Wed, 2014-11-05 at 17:36 -0800, Bjorn Andersson wrote:
[...]
> > > > > Could you be a little bit more elaborate on what you're trying to do
> > > > > and which child devices that might be?
> > > >
> > > > For example ADC drivers are required temperature compensation based
> > > > on PMIC variant and chip manufacturer.
> > > >
> > >
> > > I see, is that compensation of any practical value? Or is the
> > > compensation of academic proportions?
> >
> > It depends of what you mean by academic :-). Attached file have test
> > application which dump difference between non compensated and compensated
> > values for different temperature, manufacture and input value.
> >
[...]
> Forgot to add. PMIC subtype and version are used also in charger and BMS
> drivers to workaround hardware issues.
All of the blocks on the PM8x41 series have their own version numbers.
There's no need to look at the chip revision.
In fact, the SMBB (charger) documentation (80-NA555-12) specifically
refers to the SMBB_MISC block revision registers as the method for
determining the hardware version. The "qpnp-charger" SMBB driver in the
CAF 3.4 & 3.10 trees utilizes block specific revision numbers, as do the
"qpnp-bms" BMS driver, the "qpnp-adc-current" IADC driver, and the
"qpnp-adc-voltage" VADC driver.
The revision of the PMIC itself should be completely irrelevant to any
of the software interfacing with it.
-Courtney
On Tue, 2014-11-11 at 12:27 -0800, Courtney Cavin wrote:
> On Fri, Nov 07, 2014 at 04:40:52PM +0100, Ivan T. Ivanov wrote:
> >
> > Forgot to add. PMIC subtype and version are used also in charger and BMS
> > drivers to workaround hardware issues.
>
> All of the blocks on the PM8x41 series have their own version numbers.
> There's no need to look at the chip revision.
I am suspecting that, for whatever the reason is, after updates inside blocks,
the PMIC chip revisions update was made instead of blocks own version update.
>
> In fact, the SMBB (charger) documentation (80-NA555-12) specifically
It would be nice if I had this document :-).
> refers to the SMBB_MISC block revision registers as the method for
> determining the hardware version. The "qpnp-charger" SMBB driver in the
> CAF 3.4 & 3.10 trees utilizes block specific revision numbers, as do the
> "qpnp-bms" BMS driver, the "qpnp-adc-current" IADC driver, and the
> "qpnp-adc-voltage" VADC driver.
Hm, they read its own block revision, but they are using PMIC chip revision
for workaround decisions. What could be the reason for this?
>
> The revision of the PMIC itself should be completely irrelevant to any
> of the software interfacing with it.
>
It will be really nice if this is the case, but I am afraid it is not.
Regards,
Ivan