diff mbox series

[v4,12/12] mfd: bd9571mwv: Add support for BD9574MWF

Message ID 1608519279-13341-13-git-send-email-yoshihiro.shimoda.uh@renesas.com
State New
Headers show
Series treewide: bd9571mwv: Add support for BD9574MWF | expand

Commit Message

Yoshihiro Shimoda Dec. 21, 2020, 2:54 a.m. UTC
From: Khiem Nguyen <khiem.nguyen.xt@renesas.com>

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

Signed-off-by: Khiem Nguyen <khiem.nguyen.xt@renesas.com>
Co-developed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/mfd/bd9571mwv.c       | 83 +++++++++++++++++++++++++++++++++++++++++--
 include/linux/mfd/bd9571mwv.h | 17 +++++++--
 2 files changed, 95 insertions(+), 5 deletions(-)

Comments

Geert Uytterhoeven Dec. 22, 2020, 8:52 a.m. UTC | #1
Hi Shimoda-san,

On Mon, Dec 21, 2020 at 3:56 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> From: Khiem Nguyen <khiem.nguyen.xt@renesas.com>
>
> The new PMIC BD9574MWF inherits features from BD9571MWV.
> Add the support of new PMIC to existing bd9571mwv driver.
>
> Signed-off-by: Khiem Nguyen <khiem.nguyen.xt@renesas.com>
> Co-developed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

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
Yoshihiro Shimoda Dec. 22, 2020, 9:23 a.m. UTC | #2
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
> <yoshihiro.shimoda.uh@renesas.com> 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
Geert Uytterhoeven Dec. 22, 2020, 9:32 a.m. UTC | #3
Hi Shimoda-san,

On Tue, Dec 22, 2020 at 10:23 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> > From: Geert Uytterhoeven, Sent: Tuesday, December 22, 2020 5:53 PM
> > On Mon, Dec 21, 2020 at 3:56 AM Yoshihiro Shimoda
> > <yoshihiro.shimoda.uh@renesas.com> 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 -- geert@linux-m68k.org

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
Vaittinen, Matti Dec. 22, 2020, 9:39 a.m. UTC | #4
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
> > <yoshihiro.shimoda.uh@renesas.com> 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
Yoshihiro Shimoda Dec. 22, 2020, 10:49 a.m. UTC | #5
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
Yoshihiro Shimoda Dec. 22, 2020, 10:51 a.m. UTC | #6
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
diff mbox series

Patch

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 <marek.vasut+renesas@gmail.com>
  * 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 <marek.vasut+renesas@gmail.com>
  * 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,
 };