2020-12-21 02:56:56

by Yoshihiro Shimoda

[permalink] [raw]
Subject: [PATCH v4 12/12] mfd: bd9571mwv: Add support for BD9574MWF

From: Khiem Nguyen <[email protected]>

The new PMIC BD9574MWF inherits features from BD9571MWV.
Add the support of new PMIC to existing bd9571mwv driver.

Signed-off-by: Khiem Nguyen <[email protected]>
Co-developed-by: Yoshihiro Shimoda <[email protected]>
Signed-off-by: Yoshihiro Shimoda <[email protected]>
---
drivers/mfd/bd9571mwv.c | 83 +++++++++++++++++++++++++++++++++++++++++--
include/linux/mfd/bd9571mwv.h | 17 +++++++--
2 files changed, 95 insertions(+), 5 deletions(-)

diff --git a/drivers/mfd/bd9571mwv.c b/drivers/mfd/bd9571mwv.c
index c905ab4..ab753a9 100644
--- a/drivers/mfd/bd9571mwv.c
+++ b/drivers/mfd/bd9571mwv.c
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
- * ROHM BD9571MWV-M MFD driver
+ * ROHM BD9571MWV-M and BD9574MVF-M core driver
*
* Copyright (C) 2017 Marek Vasut <[email protected]>
* Copyright (C) 2020 Renesas Electronics Corporation
@@ -11,6 +11,7 @@
#include <linux/i2c.h>
#include <linux/interrupt.h>
#include <linux/mfd/core.h>
+#include <linux/mfd/rohm-generic.h>
#include <linux/module.h>

#include <linux/mfd/bd9571mwv.h>
@@ -118,6 +119,79 @@ static const struct bd957x_ddata bd9571mwv_ddata = {
.num_cells = ARRAY_SIZE(bd9571mwv_cells),
};

+static const struct mfd_cell bd9574mwf_cells[] = {
+ { .name = "bd9574mwf-regulator", },
+ { .name = "bd9574mwf-gpio", },
+};
+
+static const struct regmap_range bd9574mwf_readable_yes_ranges[] = {
+ regmap_reg_range(BD9571MWV_VENDOR_CODE, BD9571MWV_PRODUCT_REVISION),
+ regmap_reg_range(BD9571MWV_BKUP_MODE_CNT, BD9571MWV_BKUP_MODE_CNT),
+ regmap_reg_range(BD9571MWV_DVFS_VINIT, BD9571MWV_DVFS_SETVMAX),
+ regmap_reg_range(BD9571MWV_DVFS_SETVID, BD9571MWV_DVFS_MONIVDAC),
+ regmap_reg_range(BD9571MWV_GPIO_IN, BD9571MWV_GPIO_IN),
+ regmap_reg_range(BD9571MWV_GPIO_INT, BD9571MWV_GPIO_INTMASK),
+ regmap_reg_range(BD9571MWV_INT_INTREQ, BD9571MWV_INT_INTMASK),
+};
+
+static const struct regmap_access_table bd9574mwf_readable_table = {
+ .yes_ranges = bd9574mwf_readable_yes_ranges,
+ .n_yes_ranges = ARRAY_SIZE(bd9574mwf_readable_yes_ranges),
+};
+
+static const struct regmap_range bd9574mwf_writable_yes_ranges[] = {
+ regmap_reg_range(BD9571MWV_BKUP_MODE_CNT, BD9571MWV_BKUP_MODE_CNT),
+ regmap_reg_range(BD9571MWV_DVFS_SETVID, BD9571MWV_DVFS_SETVID),
+ regmap_reg_range(BD9571MWV_GPIO_DIR, BD9571MWV_GPIO_OUT),
+ regmap_reg_range(BD9571MWV_GPIO_INT_SET, BD9571MWV_GPIO_INTMASK),
+ regmap_reg_range(BD9571MWV_INT_INTREQ, BD9571MWV_INT_INTMASK),
+};
+
+static const struct regmap_access_table bd9574mwf_writable_table = {
+ .yes_ranges = bd9574mwf_writable_yes_ranges,
+ .n_yes_ranges = ARRAY_SIZE(bd9574mwf_writable_yes_ranges),
+};
+
+static const struct regmap_range bd9574mwf_volatile_yes_ranges[] = {
+ regmap_reg_range(BD9571MWV_DVFS_MONIVDAC, BD9571MWV_DVFS_MONIVDAC),
+ regmap_reg_range(BD9571MWV_GPIO_IN, BD9571MWV_GPIO_IN),
+ regmap_reg_range(BD9571MWV_GPIO_INT, BD9571MWV_GPIO_INT),
+ regmap_reg_range(BD9571MWV_INT_INTREQ, BD9571MWV_INT_INTREQ),
+};
+
+static const struct regmap_access_table bd9574mwf_volatile_table = {
+ .yes_ranges = bd9574mwf_volatile_yes_ranges,
+ .n_yes_ranges = ARRAY_SIZE(bd9574mwf_volatile_yes_ranges),
+};
+
+static const struct regmap_config bd9574mwf_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .cache_type = REGCACHE_RBTREE,
+ .rd_table = &bd9574mwf_readable_table,
+ .wr_table = &bd9574mwf_writable_table,
+ .volatile_table = &bd9574mwf_volatile_table,
+ .max_register = 0xff,
+};
+
+static struct regmap_irq_chip bd9574mwf_irq_chip = {
+ .name = "bd9574mwf",
+ .status_base = BD9571MWV_INT_INTREQ,
+ .mask_base = BD9571MWV_INT_INTMASK,
+ .ack_base = BD9571MWV_INT_INTREQ,
+ .init_ack_masked = true,
+ .num_regs = 1,
+ .irqs = bd9571mwv_irqs,
+ .num_irqs = ARRAY_SIZE(bd9571mwv_irqs),
+};
+
+static const struct bd957x_ddata bd9574mwf_ddata = {
+ .regmap_config = &bd9574mwf_regmap_config,
+ .irq_chip = &bd9574mwf_irq_chip,
+ .cells = bd9574mwf_cells,
+ .num_cells = ARRAY_SIZE(bd9574mwf_cells),
+};
+
static int bd957x_identify(struct device *dev, struct regmap *regmap)
{
unsigned int value;
@@ -171,6 +245,9 @@ static int bd9571mwv_probe(struct i2c_client *client,
case BD9571MWV_PRODUCT_CODE_BD9571MWV:
ddata = &bd9571mwv_ddata;
break;
+ case BD9571MWV_PRODUCT_CODE_BD9574MWF:
+ ddata = &bd9574mwf_ddata;
+ break;
default:
dev_err(dev, "Unsupported device 0x%x\n", ret);
return -ENODEV;
@@ -200,12 +277,14 @@ static int bd9571mwv_probe(struct i2c_client *client,

static const struct of_device_id bd9571mwv_of_match_table[] = {
{ .compatible = "rohm,bd9571mwv", },
+ { .compatible = "rohm,bd9574mwf", },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, bd9571mwv_of_match_table);

static const struct i2c_device_id bd9571mwv_id_table[] = {
- { "bd9571mwv", 0 },
+ { "bd9571mwv", ROHM_CHIP_TYPE_BD9571 },
+ { "bd9574mwf", ROHM_CHIP_TYPE_BD9574 },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(i2c, bd9571mwv_id_table);
diff --git a/include/linux/mfd/bd9571mwv.h b/include/linux/mfd/bd9571mwv.h
index e1716ec..8efd99d 100644
--- a/include/linux/mfd/bd9571mwv.h
+++ b/include/linux/mfd/bd9571mwv.h
@@ -1,6 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0-only */
/*
- * ROHM BD9571MWV-M driver
+ * ROHM BD9571MWV-M and BD9574MWF-M driver
*
* Copyright (C) 2017 Marek Vasut <[email protected]>
* Copyright (C) 2020 Renesas Electronics Corporation
@@ -14,11 +14,12 @@
#include <linux/device.h>
#include <linux/regmap.h>

-/* List of registers for BD9571MWV */
+/* List of registers for BD9571MWV and BD9574MWF */
#define BD9571MWV_VENDOR_CODE 0x00
#define BD9571MWV_VENDOR_CODE_VAL 0xdb
#define BD9571MWV_PRODUCT_CODE 0x01
#define BD9571MWV_PRODUCT_CODE_BD9571MWV 0x60
+#define BD9571MWV_PRODUCT_CODE_BD9574MWF 0x74
#define BD9571MWV_PRODUCT_REVISION 0x02

#define BD9571MWV_I2C_FUSA_MODE 0x10
@@ -48,6 +49,7 @@
#define BD9571MWV_VD33_VID 0x44

#define BD9571MWV_DVFS_VINIT 0x50
+#define BD9574MWF_VD09_VINIT 0x51
#define BD9571MWV_DVFS_SETVMAX 0x52
#define BD9571MWV_DVFS_BOOSTVID 0x53
#define BD9571MWV_DVFS_SETVID 0x54
@@ -61,6 +63,7 @@
#define BD9571MWV_GPIO_INT_SET 0x64
#define BD9571MWV_GPIO_INT 0x65
#define BD9571MWV_GPIO_INTMASK 0x66
+#define BD9574MWF_GPIO_MUX 0x67

#define BD9571MWV_REG_KEEP(n) (0x70 + (n))

@@ -70,6 +73,8 @@
#define BD9571MWV_PROT_ERROR_STATUS2 0x83
#define BD9571MWV_PROT_ERROR_STATUS3 0x84
#define BD9571MWV_PROT_ERROR_STATUS4 0x85
+#define BD9574MWF_PROT_ERROR_STATUS5 0x86
+#define BD9574MWF_SYSTEM_ERROR_STATUS 0x87

#define BD9571MWV_INT_INTREQ 0x90
#define BD9571MWV_INT_INTREQ_MD1_INT BIT(0)
@@ -82,6 +87,12 @@
#define BD9571MWV_INT_INTREQ_BKUP_TRG_INT BIT(7)
#define BD9571MWV_INT_INTMASK 0x91

+#define BD9574MWF_SSCG_CNT 0xA0
+#define BD9574MWF_POFFB_MRB 0xA1
+#define BD9574MWF_SMRB_WR_PROT 0xA2
+#define BD9574MWF_SMRB_ASSERT 0xA3
+#define BD9574MWF_SMRB_STATUS 0xA4
+
#define BD9571MWV_ACCESS_KEY 0xff

/* Define the BD9571MWV IRQ numbers */
@@ -91,7 +102,7 @@ enum bd9571mwv_irqs {
BD9571MWV_IRQ_MD2_E2,
BD9571MWV_IRQ_PROT_ERR,
BD9571MWV_IRQ_GP,
- BD9571MWV_IRQ_128H_OF,
+ BD9571MWV_IRQ_128H_OF, /* BKUP_HOLD on BD9574MWF */
BD9571MWV_IRQ_WDT_OF,
BD9571MWV_IRQ_BKUP_TRG,
};
--
2.7.4


2020-12-22 08:55:08

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v4 12/12] mfd: bd9571mwv: Add support for BD9574MWF

Hi Shimoda-san,

On Mon, Dec 21, 2020 at 3:56 AM Yoshihiro Shimoda
<[email protected]> wrote:
> From: Khiem Nguyen <[email protected]>
>
> The new PMIC BD9574MWF inherits features from BD9571MWV.
> Add the support of new PMIC to existing bd9571mwv driver.
>
> Signed-off-by: Khiem Nguyen <[email protected]>
> Co-developed-by: Yoshihiro Shimoda <[email protected]>
> Signed-off-by: Yoshihiro Shimoda <[email protected]>

Thanks for your patch!

> --- a/drivers/mfd/bd9571mwv.c
> +++ b/drivers/mfd/bd9571mwv.c

> @@ -200,12 +277,14 @@ static int bd9571mwv_probe(struct i2c_client *client,
>
> static const struct of_device_id bd9571mwv_of_match_table[] = {
> { .compatible = "rohm,bd9571mwv", },
> + { .compatible = "rohm,bd9574mwf", },
> { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, bd9571mwv_of_match_table);
>
> static const struct i2c_device_id bd9571mwv_id_table[] = {
> - { "bd9571mwv", 0 },
> + { "bd9571mwv", ROHM_CHIP_TYPE_BD9571 },
> + { "bd9574mwf", ROHM_CHIP_TYPE_BD9574 },

Why add the chip types? These are unused, and the driver uses
autodetection of the chip variant anyway.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2020-12-22 09:25:51

by Yoshihiro Shimoda

[permalink] [raw]
Subject: RE: [PATCH v4 12/12] mfd: bd9571mwv: Add support for BD9574MWF

Hi Geert-san,

Thank you for your review!

> From: Geert Uytterhoeven, Sent: Tuesday, December 22, 2020 5:53 PM
> On Mon, Dec 21, 2020 at 3:56 AM Yoshihiro Shimoda
> <[email protected]> wrote:
<snip>
> > --- a/drivers/mfd/bd9571mwv.c
> > +++ b/drivers/mfd/bd9571mwv.c
>
> > @@ -200,12 +277,14 @@ static int bd9571mwv_probe(struct i2c_client *client,
> >
> > static const struct of_device_id bd9571mwv_of_match_table[] = {
> > { .compatible = "rohm,bd9571mwv", },
> > + { .compatible = "rohm,bd9574mwf", },
> > { /* sentinel */ }
> > };
> > MODULE_DEVICE_TABLE(of, bd9571mwv_of_match_table);
> >
> > static const struct i2c_device_id bd9571mwv_id_table[] = {
> > - { "bd9571mwv", 0 },
> > + { "bd9571mwv", ROHM_CHIP_TYPE_BD9571 },
> > + { "bd9574mwf", ROHM_CHIP_TYPE_BD9574 },
>
> Why add the chip types? These are unused, and the driver uses
> autodetection of the chip variant anyway.

I just added the chip types in the future use. As you said,
these are unused and we should not add a future use in general.
So, I'll remove this change.

Also, I think I should remove the following patch.

[PATCH v4 08/12] gpio: bd9571mwv: Add BD9574MWF support

Best regards,
Yoshihiro Shimoda

2020-12-22 09:34:00

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v4 12/12] mfd: bd9571mwv: Add support for BD9574MWF

Hi Shimoda-san,

On Tue, Dec 22, 2020 at 10:23 AM Yoshihiro Shimoda
<[email protected]> wrote:
> > From: Geert Uytterhoeven, Sent: Tuesday, December 22, 2020 5:53 PM
> > On Mon, Dec 21, 2020 at 3:56 AM Yoshihiro Shimoda
> > <[email protected]> wrote:
> <snip>
> > > --- a/drivers/mfd/bd9571mwv.c
> > > +++ b/drivers/mfd/bd9571mwv.c
> >
> > > @@ -200,12 +277,14 @@ static int bd9571mwv_probe(struct i2c_client *client,
> > >
> > > static const struct of_device_id bd9571mwv_of_match_table[] = {
> > > { .compatible = "rohm,bd9571mwv", },
> > > + { .compatible = "rohm,bd9574mwf", },
> > > { /* sentinel */ }
> > > };
> > > MODULE_DEVICE_TABLE(of, bd9571mwv_of_match_table);
> > >
> > > static const struct i2c_device_id bd9571mwv_id_table[] = {
> > > - { "bd9571mwv", 0 },
> > > + { "bd9571mwv", ROHM_CHIP_TYPE_BD9571 },
> > > + { "bd9574mwf", ROHM_CHIP_TYPE_BD9574 },
> >
> > Why add the chip types? These are unused, and the driver uses
> > autodetection of the chip variant anyway.
>
> I just added the chip types in the future use. As you said,
> these are unused and we should not add a future use in general.
> So, I'll remove this change.

OK.

> Also, I think I should remove the following patch.
>
> [PATCH v4 08/12] gpio: bd9571mwv: Add BD9574MWF support

You mean removing the chip types from bd9571mwv_gpio_id_table[]?
You still need the "bd9574mwf-gpio" entry, don't you?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2020-12-22 09:43:36

by Matti Vaittinen

[permalink] [raw]
Subject: Re: [PATCH v4 12/12] mfd: bd9571mwv: Add support for BD9574MWF


On Tue, 2020-12-22 at 09:23 +0000, Yoshihiro Shimoda wrote:
> Hi Geert-san,
>
> Thank you for your review!
>
> > From: Geert Uytterhoeven, Sent: Tuesday, December 22, 2020 5:53 PM
> > On Mon, Dec 21, 2020 at 3:56 AM Yoshihiro Shimoda
> > <[email protected]> wrote:
> <snip>
> > > --- a/drivers/mfd/bd9571mwv.c
> > > +++ b/drivers/mfd/bd9571mwv.c
> > > @@ -200,12 +277,14 @@ static int bd9571mwv_probe(struct
> > > i2c_client *client,
> > >
> > > static const struct of_device_id bd9571mwv_of_match_table[] = {
> > > { .compatible = "rohm,bd9571mwv", },
> > > + { .compatible = "rohm,bd9574mwf", },
> > > { /* sentinel */ }
> > > };
> > > MODULE_DEVICE_TABLE(of, bd9571mwv_of_match_table);
> > >
> > > static const struct i2c_device_id bd9571mwv_id_table[] = {
> > > - { "bd9571mwv", 0 },
> > > + { "bd9571mwv", ROHM_CHIP_TYPE_BD9571 },
> > > + { "bd9574mwf", ROHM_CHIP_TYPE_BD9574 },
> >
> > Why add the chip types? These are unused, and the driver uses
> > autodetection of the chip variant anyway.
>
> I just added the chip types in the future use. As you said,
> these are unused and we should not add a future use in general.
> So, I'll remove this change.
>
> Also, I think I should remove the following patch.
>
> [PATCH v4 08/12] gpio: bd9571mwv: Add BD9574MWF support

I think this depends on whether you wish to add support for the

> "RECOV_GPOUT", "FREQSEL" and "RTC_IN",

which you mention in GPIO commit message. If you plan on adding the
support, then you need to differentiate the ICs on GPIO driver, right?

Best Regards
Matti Vaittinen

2020-12-22 10:53:14

by Yoshihiro Shimoda

[permalink] [raw]
Subject: RE: [PATCH v4 12/12] mfd: bd9571mwv: Add support for BD9574MWF

Hi Geert-san,

> From: Geert Uytterhoeven, Sent: Tuesday, December 22, 2020 6:32 PM
<snip>
> > Also, I think I should remove the following patch.
> >
> > [PATCH v4 08/12] gpio: bd9571mwv: Add BD9574MWF support
>
> You mean removing the chip types from bd9571mwv_gpio_id_table[]?
> You still need the "bd9574mwf-gpio" entry, don't you?

You're correct. The MFD driver uses "bd9574mwf-gpio". And,
"bd9574mwf-gpio" with 0 is not good. So, I'll keep this patch.
Thank you for the point it out.

Best regards,
Yoshihiro Shimoda

2020-12-22 10:56:16

by Yoshihiro Shimoda

[permalink] [raw]
Subject: RE: [PATCH v4 12/12] mfd: bd9571mwv: Add support for BD9574MWF

Hi Matti-san,

> From: Vaittinen, Matti, Sent: Tuesday, December 22, 2020 6:39 PM
<snip>
> > Also, I think I should remove the following patch.
> >
> > [PATCH v4 08/12] gpio: bd9571mwv: Add BD9574MWF support
>
> I think this depends on whether you wish to add support for the
>
> > "RECOV_GPOUT", "FREQSEL" and "RTC_IN",
>
> which you mention in GPIO commit message. If you plan on adding the
> support, then you need to differentiate the ICs on GPIO driver, right?

Thank you for the reply.
As I replied to Geert-san, at least this MFD driver uses "bd9574mwf-gpio"
for probing. So, I'll keep that patch as-is.

Best regards,
Yoshihiro Shimoda