The BCM590xx PMUs have two I2C registers for reading the PMU ID
and revision. The revision is useful for subdevice drivers, since
different revisions may have slight differences in behavior (for
example - BCM59054 has different regulator configurations for
revision A0 and A1).
Check the PMU ID register and make sure it matches the DT compatible.
Fetch the digital and analog revision from the PMUREV register
so that it can be used in subdevice drivers.
Also add some known revision values to bcm590xx.h, for convenience
when writing subdevice drivers.
Signed-off-by: Artur Weber <aweber.kernel@gmail.com>
---
Changes in v8:
- Change PMU ID print from dev_info to dev_dbg
- Unwrap some lines and let them go up to 100 chars
- Drop comment above bcm590xx_parse_version (keep the comments inside
the function, they make it a bit more clear what's happening since
it's doing two separate things)
Changes in v7:
- Return -ENODEV on PMU ID mismatch
- Drop "Check your DT compatible" from ID mismatch error message
Changes in v6:
- Adapt to PMUID being passed as device type value
- Rename rev_dig and rev_ana to rev_digital and rev_analog
- Rewrite commit message
Changes in v5:
- Add REG_ prefix to register offset constant names
Changes in v4:
- Added this commit
---
drivers/mfd/bcm590xx.c | 55 ++++++++++++++++++++++++++++++++++++++++++++
include/linux/mfd/bcm590xx.h | 14 +++++++++++
2 files changed, 69 insertions(+)
diff --git a/drivers/mfd/bcm590xx.c b/drivers/mfd/bcm590xx.c
index 4620eed0066fbf1dd691a2e392e967747b4d125b..12d0db4237e79fcfcb2af4a0d93961b6239a3863 100644
--- a/drivers/mfd/bcm590xx.c
+++ b/drivers/mfd/bcm590xx.c
@@ -17,6 +17,15 @@
#include <linux/regmap.h>
#include <linux/slab.h>
+/* Under primary I2C address: */
+#define BCM590XX_REG_PMUID 0x1e
+
+#define BCM590XX_REG_PMUREV 0x1f
+#define BCM590XX_PMUREV_DIG_MASK 0xF
+#define BCM590XX_PMUREV_DIG_SHIFT 0
+#define BCM590XX_PMUREV_ANA_MASK 0xF0
+#define BCM590XX_PMUREV_ANA_SHIFT 4
+
static const struct mfd_cell bcm590xx_devs[] = {
{
.name = "bcm590xx-vregs",
@@ -37,6 +46,48 @@ static const struct regmap_config bcm590xx_regmap_config_sec = {
.cache_type = REGCACHE_MAPLE,
};
+/* Map PMU ID value to model name string */
+static const char * const bcm590xx_names[] = {
+ [BCM590XX_PMUID_BCM59054] = "BCM59054",
+ [BCM590XX_PMUID_BCM59056] = "BCM59056",
+};
+
+static int bcm590xx_parse_version(struct bcm590xx *bcm590xx)
+{
+ unsigned int id, rev;
+ int ret;
+
+ /* Get PMU ID and verify that it matches compatible */
+ ret = regmap_read(bcm590xx->regmap_pri, BCM590XX_REG_PMUID, &id);
+ if (ret) {
+ dev_err(bcm590xx->dev, "failed to read PMU ID: %d\n", ret);
+ return ret;
+ }
+
+ if (id != bcm590xx->pmu_id) {
+ dev_err(bcm590xx->dev, "Incorrect ID for %s: expected %x, got %x.\n",
+ bcm590xx_names[bcm590xx->pmu_id], bcm590xx->pmu_id, id);
+ return -ENODEV;
+ }
+
+ /* Get PMU revision and store it in the info struct */
+ ret = regmap_read(bcm590xx->regmap_pri, BCM590XX_REG_PMUREV, &rev);
+ if (ret) {
+ dev_err(bcm590xx->dev, "failed to read PMU revision: %d\n", ret);
+ return ret;
+ }
+
+ bcm590xx->rev_digital = (rev & BCM590XX_PMUREV_DIG_MASK) >> BCM590XX_PMUREV_DIG_SHIFT;
+
+ bcm590xx->rev_analog = (rev & BCM590XX_PMUREV_ANA_MASK) >> BCM590XX_PMUREV_ANA_SHIFT;
+
+ dev_dbg(bcm590xx->dev, "PMU ID 0x%x (%s), revision: digital %d, analog %d",
+ id, bcm590xx_names[id],
+ bcm590xx->rev_digital, bcm590xx->rev_analog);
+
+ return 0;
+}
+
static int bcm590xx_i2c_probe(struct i2c_client *i2c_pri)
{
struct bcm590xx *bcm590xx;
@@ -78,6 +129,10 @@ static int bcm590xx_i2c_probe(struct i2c_client *i2c_pri)
goto err;
}
+ ret = bcm590xx_parse_version(bcm590xx);
+ if (ret)
+ goto err;
+
ret = devm_mfd_add_devices(&i2c_pri->dev, -1, bcm590xx_devs,
ARRAY_SIZE(bcm590xx_devs), NULL, 0, NULL);
if (ret < 0) {
diff --git a/include/linux/mfd/bcm590xx.h b/include/linux/mfd/bcm590xx.h
index 8d146e3b102a7dbce6f4dbab9f8ae5a9c4e68c0e..fbc458e94bef923ca1b69afe2cac944adf6fedf8 100644
--- a/include/linux/mfd/bcm590xx.h
+++ b/include/linux/mfd/bcm590xx.h
@@ -17,6 +17,16 @@
#define BCM590XX_PMUID_BCM59054 0x54
#define BCM590XX_PMUID_BCM59056 0x56
+/* Known chip revision IDs */
+#define BCM59054_REV_DIGITAL_A1 1
+#define BCM59054_REV_ANALOG_A1 2
+
+#define BCM59056_REV_DIGITAL_A0 1
+#define BCM59056_REV_ANALOG_A0 1
+
+#define BCM59056_REV_DIGITAL_B0 2
+#define BCM59056_REV_ANALOG_B0 2
+
/* max register address */
#define BCM590XX_MAX_REGISTER_PRI 0xe7
#define BCM590XX_MAX_REGISTER_SEC 0xf0
@@ -30,6 +40,10 @@ struct bcm590xx {
/* PMU ID value; also used as device type */
u8 pmu_id;
+
+ /* Chip revision, read from PMUREV reg */
+ u8 rev_digital;
+ u8 rev_analog;
};
#endif /* __LINUX_MFD_BCM590XX_H */
--
2.49.0On Wed, 30 Apr 2025, Artur Weber wrote:
> The BCM590xx PMUs have two I2C registers for reading the PMU ID
> and revision. The revision is useful for subdevice drivers, since
> different revisions may have slight differences in behavior (for
> example - BCM59054 has different regulator configurations for
> revision A0 and A1).
>
> Check the PMU ID register and make sure it matches the DT compatible.
> Fetch the digital and analog revision from the PMUREV register
> so that it can be used in subdevice drivers.
>
> Also add some known revision values to bcm590xx.h, for convenience
> when writing subdevice drivers.
>
> Signed-off-by: Artur Weber <aweber.kernel@gmail.com>
> ---
> Changes in v8:
> - Change PMU ID print from dev_info to dev_dbg
> - Unwrap some lines and let them go up to 100 chars
> - Drop comment above bcm590xx_parse_version (keep the comments inside
> the function, they make it a bit more clear what's happening since
> it's doing two separate things)
>
> Changes in v7:
> - Return -ENODEV on PMU ID mismatch
> - Drop "Check your DT compatible" from ID mismatch error message
>
> Changes in v6:
> - Adapt to PMUID being passed as device type value
> - Rename rev_dig and rev_ana to rev_digital and rev_analog
> - Rewrite commit message
>
> Changes in v5:
> - Add REG_ prefix to register offset constant names
>
> Changes in v4:
> - Added this commit
> ---
> drivers/mfd/bcm590xx.c | 55 ++++++++++++++++++++++++++++++++++++++++++++
> include/linux/mfd/bcm590xx.h | 14 +++++++++++
> 2 files changed, 69 insertions(+)
>
> diff --git a/drivers/mfd/bcm590xx.c b/drivers/mfd/bcm590xx.c
> index 4620eed0066fbf1dd691a2e392e967747b4d125b..12d0db4237e79fcfcb2af4a0d93961b6239a3863 100644
> --- a/drivers/mfd/bcm590xx.c
> +++ b/drivers/mfd/bcm590xx.c
> @@ -17,6 +17,15 @@
> #include <linux/regmap.h>
> #include <linux/slab.h>
>
> +/* Under primary I2C address: */
> +#define BCM590XX_REG_PMUID 0x1e
> +
> +#define BCM590XX_REG_PMUREV 0x1f
> +#define BCM590XX_PMUREV_DIG_MASK 0xF
> +#define BCM590XX_PMUREV_DIG_SHIFT 0
> +#define BCM590XX_PMUREV_ANA_MASK 0xF0
> +#define BCM590XX_PMUREV_ANA_SHIFT 4
> +
> static const struct mfd_cell bcm590xx_devs[] = {
> {
> .name = "bcm590xx-vregs",
> @@ -37,6 +46,48 @@ static const struct regmap_config bcm590xx_regmap_config_sec = {
> .cache_type = REGCACHE_MAPLE,
> };
>
> +/* Map PMU ID value to model name string */
> +static const char * const bcm590xx_names[] = {
> + [BCM590XX_PMUID_BCM59054] = "BCM59054",
> + [BCM590XX_PMUID_BCM59056] = "BCM59056",
> +};
> +
> +static int bcm590xx_parse_version(struct bcm590xx *bcm590xx)
> +{
> + unsigned int id, rev;
> + int ret;
> +
> + /* Get PMU ID and verify that it matches compatible */
> + ret = regmap_read(bcm590xx->regmap_pri, BCM590XX_REG_PMUID, &id);
> + if (ret) {
> + dev_err(bcm590xx->dev, "failed to read PMU ID: %d\n", ret);
> + return ret;
> + }
> +
> + if (id != bcm590xx->pmu_id) {
> + dev_err(bcm590xx->dev, "Incorrect ID for %s: expected %x, got %x.\n",
> + bcm590xx_names[bcm590xx->pmu_id], bcm590xx->pmu_id, id);
> + return -ENODEV;
> + }
> +
> + /* Get PMU revision and store it in the info struct */
> + ret = regmap_read(bcm590xx->regmap_pri, BCM590XX_REG_PMUREV, &rev);
> + if (ret) {
> + dev_err(bcm590xx->dev, "failed to read PMU revision: %d\n", ret);
> + return ret;
> + }
> +
> + bcm590xx->rev_digital = (rev & BCM590XX_PMUREV_DIG_MASK) >> BCM590XX_PMUREV_DIG_SHIFT;
> +
> + bcm590xx->rev_analog = (rev & BCM590XX_PMUREV_ANA_MASK) >> BCM590XX_PMUREV_ANA_SHIFT;
> +
> + dev_dbg(bcm590xx->dev, "PMU ID 0x%x (%s), revision: digital %d, analog %d",
> + id, bcm590xx_names[id],
> + bcm590xx->rev_digital, bcm590xx->rev_analog);
Nit: This would fit on the line above.
> +
> + return 0;
> +}
> +
> static int bcm590xx_i2c_probe(struct i2c_client *i2c_pri)
> {
> struct bcm590xx *bcm590xx;
> @@ -78,6 +129,10 @@ static int bcm590xx_i2c_probe(struct i2c_client *i2c_pri)
> goto err;
> }
>
> + ret = bcm590xx_parse_version(bcm590xx);
> + if (ret)
> + goto err;
> +
> ret = devm_mfd_add_devices(&i2c_pri->dev, -1, bcm590xx_devs,
> ARRAY_SIZE(bcm590xx_devs), NULL, 0, NULL);
> if (ret < 0) {
> diff --git a/include/linux/mfd/bcm590xx.h b/include/linux/mfd/bcm590xx.h
> index 8d146e3b102a7dbce6f4dbab9f8ae5a9c4e68c0e..fbc458e94bef923ca1b69afe2cac944adf6fedf8 100644
> --- a/include/linux/mfd/bcm590xx.h
> +++ b/include/linux/mfd/bcm590xx.h
> @@ -17,6 +17,16 @@
> #define BCM590XX_PMUID_BCM59054 0x54
> #define BCM590XX_PMUID_BCM59056 0x56
>
> +/* Known chip revision IDs */
> +#define BCM59054_REV_DIGITAL_A1 1
> +#define BCM59054_REV_ANALOG_A1 2
> +
> +#define BCM59056_REV_DIGITAL_A0 1
> +#define BCM59056_REV_ANALOG_A0 1
> +
> +#define BCM59056_REV_DIGITAL_B0 2
> +#define BCM59056_REV_ANALOG_B0 2
> +
> /* max register address */
> #define BCM590XX_MAX_REGISTER_PRI 0xe7
> #define BCM590XX_MAX_REGISTER_SEC 0xf0
> @@ -30,6 +40,10 @@ struct bcm590xx {
>
> /* PMU ID value; also used as device type */
> u8 pmu_id;
> +
> + /* Chip revision, read from PMUREV reg */
> + u8 rev_digital;
> + u8 rev_analog;
> };
>
> #endif /* __LINUX_MFD_BCM590XX_H */
>
> --
> 2.49.0
>
--
Lee Jones [李琼斯]
Hi Artur, one note below. On Wed, Apr 30, 2025 at 09:07:09AM +0200, Artur Weber wrote: > The BCM590xx PMUs have two I2C registers for reading the PMU ID > and revision. The revision is useful for subdevice drivers, since > different revisions may have slight differences in behavior (for > example - BCM59054 has different regulator configurations for > revision A0 and A1). > > Check the PMU ID register and make sure it matches the DT compatible. > Fetch the digital and analog revision from the PMUREV register > so that it can be used in subdevice drivers. > > Also add some known revision values to bcm590xx.h, for convenience > when writing subdevice drivers. > > Signed-off-by: Artur Weber <aweber.kernel@gmail.com> > --- [snip] > diff --git a/include/linux/mfd/bcm590xx.h b/include/linux/mfd/bcm590xx.h > index 8d146e3b102a7dbce6f4dbab9f8ae5a9c4e68c0e..fbc458e94bef923ca1b69afe2cac944adf6fedf8 100644 > --- a/include/linux/mfd/bcm590xx.h > +++ b/include/linux/mfd/bcm590xx.h > @@ -17,6 +17,16 @@ > #define BCM590XX_PMUID_BCM59054 0x54 > #define BCM590XX_PMUID_BCM59056 0x56 > > +/* Known chip revision IDs */ > +#define BCM59054_REV_DIGITAL_A1 1 1 seems to be the digital revision ID for A0 (couldn't find the analog revision ID), see [1]. Other values seems to match downstream (as far as I can tell anyway). [1] https://github.com/Samsung-KYLEPROXX/android_kernel_samsung_kyleproxx/blob/cm-13.0/include/linux/mfd/bcmpmu59xxx.h#L82 Regards, Stanislav > +#define BCM59054_REV_ANALOG_A1 2 > + > +#define BCM59056_REV_DIGITAL_A0 1 > +#define BCM59056_REV_ANALOG_A0 1 > + > +#define BCM59056_REV_DIGITAL_B0 2 > +#define BCM59056_REV_ANALOG_B0 2 > +
On 5/6/25 18:11, Stanislav Jakubek wrote: > Hi Artur, > one note below. > > On Wed, Apr 30, 2025 at 09:07:09AM +0200, Artur Weber wrote: >> The BCM590xx PMUs have two I2C registers for reading the PMU ID >> and revision. The revision is useful for subdevice drivers, since >> different revisions may have slight differences in behavior (for >> example - BCM59054 has different regulator configurations for >> revision A0 and A1). >> >> Check the PMU ID register and make sure it matches the DT compatible. >> Fetch the digital and analog revision from the PMUREV register >> so that it can be used in subdevice drivers. >> >> Also add some known revision values to bcm590xx.h, for convenience >> when writing subdevice drivers. >> >> Signed-off-by: Artur Weber <aweber.kernel@gmail.com> >> --- > > [snip] > >> diff --git a/include/linux/mfd/bcm590xx.h b/include/linux/mfd/bcm590xx.h >> index 8d146e3b102a7dbce6f4dbab9f8ae5a9c4e68c0e..fbc458e94bef923ca1b69afe2cac944adf6fedf8 100644 >> --- a/include/linux/mfd/bcm590xx.h >> +++ b/include/linux/mfd/bcm590xx.h >> @@ -17,6 +17,16 @@ >> #define BCM590XX_PMUID_BCM59054 0x54 >> #define BCM590XX_PMUID_BCM59056 0x56 >> >> +/* Known chip revision IDs */ >> +#define BCM59054_REV_DIGITAL_A1 1 > > 1 seems to be the digital revision ID for A0 (couldn't find the analog > revision ID), see [1]. > > Other values seems to match downstream (as far as I can tell anyway). > > [1] https://github.com/Samsung-KYLEPROXX/android_kernel_samsung_kyleproxx/blob/cm-13.0/include/linux/mfd/bcmpmu59xxx.h#L82 From my testing on a device with the BCM59054A1 (BCM23550-based Samsung Galaxy Grand Neo), the digital value is also 1 on this model: bcm590xx 0-0008: PMU ID 0x54 (BCM59054), revision: dig 1 ana 2 (This constant is not actually used anywhere in code yet - I just included it for the sake of completeness, since the BCM59056 headers in downstream listed both values...) Best regards Artur
On Tue, May 06, 2025 at 09:03:15PM +0200, Artur Weber wrote: > On 5/6/25 18:11, Stanislav Jakubek wrote: > > Hi Artur, > > one note below. > > > > On Wed, Apr 30, 2025 at 09:07:09AM +0200, Artur Weber wrote: > > > The BCM590xx PMUs have two I2C registers for reading the PMU ID > > > and revision. The revision is useful for subdevice drivers, since > > > different revisions may have slight differences in behavior (for > > > example - BCM59054 has different regulator configurations for > > > revision A0 and A1). > > > > > > Check the PMU ID register and make sure it matches the DT compatible. > > > Fetch the digital and analog revision from the PMUREV register > > > so that it can be used in subdevice drivers. > > > > > > Also add some known revision values to bcm590xx.h, for convenience > > > when writing subdevice drivers. > > > > > > Signed-off-by: Artur Weber <aweber.kernel@gmail.com> > > > --- > > > > [snip] > > > > > diff --git a/include/linux/mfd/bcm590xx.h b/include/linux/mfd/bcm590xx.h > > > index 8d146e3b102a7dbce6f4dbab9f8ae5a9c4e68c0e..fbc458e94bef923ca1b69afe2cac944adf6fedf8 100644 > > > --- a/include/linux/mfd/bcm590xx.h > > > +++ b/include/linux/mfd/bcm590xx.h > > > @@ -17,6 +17,16 @@ > > > #define BCM590XX_PMUID_BCM59054 0x54 > > > #define BCM590XX_PMUID_BCM59056 0x56 > > > +/* Known chip revision IDs */ > > > +#define BCM59054_REV_DIGITAL_A1 1 > > > > 1 seems to be the digital revision ID for A0 (couldn't find the analog > > revision ID), see [1]. > > > > Other values seems to match downstream (as far as I can tell anyway). > > > > [1] https://github.com/Samsung-KYLEPROXX/android_kernel_samsung_kyleproxx/blob/cm-13.0/include/linux/mfd/bcmpmu59xxx.h#L82 > From my testing on a device with the BCM59054A1 (BCM23550-based Samsung > Galaxy Grand Neo), the digital value is also 1 on this model: > > bcm590xx 0-0008: PMU ID 0x54 (BCM59054), revision: dig 1 ana 2 > > (This constant is not actually used anywhere in code yet - I just > included it for the sake of completeness, since the BCM59056 headers > in downstream listed both values...) > > Best regards > Artur Thanks for checking Artur! I guess both BCM59054 A0 and A1 have the same digital revision? Well, to be sure we'd have to get the hardware, since downstream doesn't use this value either. So I guess the patch is good as-is, we can add known IDs when we... know them ;) Regards, Stanislav
© 2016 - 2026 Red Hat, Inc.