Re: [PATCH v6 06/10] mfd: bcm590xx: Add PMU ID/revision parsing function

Lee Jones
2025-03-13T13:20:41Z

On Tue, 04 Mar 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 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 | 63 ++++++++++++++++++++++++++++++++++++++++++++
> include/linux/mfd/bcm590xx.h | 14 ++++++++++
> 2 files changed, 77 insertions(+)
>
> diff --git a/drivers/mfd/bcm590xx.c b/drivers/mfd/bcm590xx.c
> index 4620eed0066fbf1dd691a2e392e967747b4d125b..74dc4ae5ecd5db7fadc56918f63110c1265d4a76 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,56 @@ 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",
> +};
> +
> +/*
> + * Parse the version from version registers and make sure it matches
> + * the device type passed to the compatible.
> + */
> +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. Check your DT compatible.\n",

Isn't it more likely that the H/W this is being executed on is
unsupported? If so, say that instead.

> + bcm590xx_names[bcm590xx->pmu_id], bcm590xx->pmu_id, id);
> + return -EINVAL;

-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_info(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 +137,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.48.1
>

--
Lee Jones [李琼斯]