[v8,4/5] rtc: mt6397: Add support for the MediaTek MT6358 RTC
diff mbox series

Message ID 1580730044-30501-5-git-send-email-hsin-hsiung.wang@mediatek.com
State New
Headers show
Series
  • Add Support for MediaTek PMIC MT6358
Related show

Commit Message

Hsin-hsiung Wang Feb. 3, 2020, 11:40 a.m. UTC
From: Ran Bi <ran.bi@mediatek.com>

This add support for the MediaTek MT6358 RTC. Driver using
compatible data to store different RTC_WRTGR address offset.

Signed-off-by: Ran Bi <ran.bi@mediatek.com>
Signed-off-by: Hsin-Hsiung Wang <hsin-hsiung.wang@mediatek.com>
---
 drivers/rtc/rtc-mt6397.c       | 25 +++++++++++++++++--------
 include/linux/mfd/mt6397/rtc.h | 16 +++++++++++++++-
 2 files changed, 32 insertions(+), 9 deletions(-)

Comments

Yingjoe Chen Feb. 3, 2020, 4:50 p.m. UTC | #1
On Mon, 2020-02-03 at 19:40 +0800, Hsin-Hsiung Wang wrote:
> From: Ran Bi <ran.bi@mediatek.com>
> 
> This add support for the MediaTek MT6358 RTC. Driver using
> compatible data to store different RTC_WRTGR address offset.
> 
> Signed-off-by: Ran Bi <ran.bi@mediatek.com>
> Signed-off-by: Hsin-Hsiung Wang <hsin-hsiung.wang@mediatek.com>
> ---
>  drivers/rtc/rtc-mt6397.c       | 25 +++++++++++++++++--------
>  include/linux/mfd/mt6397/rtc.h | 16 +++++++++++++++-
>  2 files changed, 32 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
> index 5249fc9..a90735e1 100644
> --- a/drivers/rtc/rtc-mt6397.c
> +++ b/drivers/rtc/rtc-mt6397.c
> @@ -9,18 +9,31 @@
>  #include <linux/mfd/mt6397/core.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
> +#include <linux/of_device.h>
>  #include <linux/platform_device.h>
>  #include <linux/regmap.h>
>  #include <linux/rtc.h>
>  #include <linux/mfd/mt6397/rtc.h>
>  #include <linux/mod_devicetable.h>
>  
> +static const struct of_device_id mt6397_rtc_of_match[] = {
> +	{ .compatible = "mediatek,mt6323-rtc",
> +		.data = (void *)&mt6397_rtc_data, },
> +	{ .compatible = "mediatek,mt6358-rtc",
> +		.data = (void *)&mt6358_rtc_data, },
> +	{ .compatible = "mediatek,mt6397-rtc",
> +		.data = (void *)&mt6397_rtc_data, },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, mt6397_rtc_of_match);
> +
>  static int mtk_rtc_write_trigger(struct mt6397_rtc *rtc)
>  {
>  	int ret;
>  	u32 data;
>  
> -	ret = regmap_write(rtc->regmap, rtc->addr_base + RTC_WRTGR, 1);
> +	ret = regmap_write(rtc->regmap,
> +			   rtc->addr_base + rtc->data->wrtgr, 1);

nit: fit in one line.

<...>

> diff --git a/include/linux/mfd/mt6397/rtc.h b/include/linux/mfd/mt6397/rtc.h
> index f84b916..fffe34a 100644
> --- a/include/linux/mfd/mt6397/rtc.h
> +++ b/include/linux/mfd/mt6397/rtc.h
> @@ -18,7 +18,8 @@
>  #define RTC_BBPU_CBUSY         BIT(6)
>  #define RTC_BBPU_KEY            (0x43 << 8)
>  
> -#define RTC_WRTGR              0x003c
> +#define RTC_WRTGR_MT6358       0x3a
> +#define RTC_WRTGR_MT6397       0x3c
>  
>  #define RTC_IRQ_STA            0x0002
>  #define RTC_IRQ_STA_AL         BIT(0)
> @@ -57,6 +58,10 @@
>  #define MTK_RTC_POLL_DELAY_US  10
>  #define MTK_RTC_POLL_TIMEOUT   (jiffies_to_usecs(HZ))
>  
> +struct mtk_rtc_data {
> +	u32			wrtgr;
> +};
> +
>  struct mt6397_rtc {
>  	struct device           *dev;
>  	struct rtc_device       *rtc_dev;
> @@ -66,6 +71,15 @@ struct mt6397_rtc {
>  	struct regmap           *regmap;
>  	int                     irq;
>  	u32                     addr_base;
> +	const struct mtk_rtc_data *data;
> +};
> +
> +static const struct mtk_rtc_data mt6358_rtc_data = {
> +	.wrtgr = RTC_WRTGR_MT6358,
> +};
> +
> +static const struct mtk_rtc_data mt6397_rtc_data = {
> +	.wrtgr = RTC_WRTGR_MT6397,
>  };

Hi,

Putting these in header file doesn't looks right to me.
Who need this? can you move them back to rtc-mt6397.c?

Joe.C
Ran Bi Feb. 17, 2020, 3:51 a.m. UTC | #2
On Tue, 2020-02-04 at 00:50 +0800, Yingjoe Chen wrote:
> > diff --git a/include/linux/mfd/mt6397/rtc.h b/include/linux/mfd/mt6397/rtc.h
> > index f84b916..fffe34a 100644
> > --- a/include/linux/mfd/mt6397/rtc.h
> > +++ b/include/linux/mfd/mt6397/rtc.h
> > @@ -18,7 +18,8 @@
> >  #define RTC_BBPU_CBUSY         BIT(6)
> >  #define RTC_BBPU_KEY            (0x43 << 8)
> >  
> > -#define RTC_WRTGR              0x003c
> > +#define RTC_WRTGR_MT6358       0x3a
> > +#define RTC_WRTGR_MT6397       0x3c
> >  
> >  #define RTC_IRQ_STA            0x0002
> >  #define RTC_IRQ_STA_AL         BIT(0)
> > @@ -57,6 +58,10 @@
> >  #define MTK_RTC_POLL_DELAY_US  10
> >  #define MTK_RTC_POLL_TIMEOUT   (jiffies_to_usecs(HZ))
> >  
> > +struct mtk_rtc_data {
> > +	u32			wrtgr;
> > +};
> > +
> >  struct mt6397_rtc {
> >  	struct device           *dev;
> >  	struct rtc_device       *rtc_dev;
> > @@ -66,6 +71,15 @@ struct mt6397_rtc {
> >  	struct regmap           *regmap;
> >  	int                     irq;
> >  	u32                     addr_base;
> > +	const struct mtk_rtc_data *data;
> > +};
> > +
> > +static const struct mtk_rtc_data mt6358_rtc_data = {
> > +	.wrtgr = RTC_WRTGR_MT6358,
> > +};
> > +
> > +static const struct mtk_rtc_data mt6397_rtc_data = {
> > +	.wrtgr = RTC_WRTGR_MT6397,
> >  };
> 
> Hi,
> 
> Putting these in header file doesn't looks right to me.
> Who need this? can you move them back to rtc-mt6397.c?
> 
> Joe.C
> 

This could also effect kernel/drivers/power/reset/mt6323-poweroff.c
which using same region of RTC registers.
There are 2 ways of modification:
1. kernel/drivers/rtc/rtc-mt6397.c implement do_pwroff function and
export to mt6323-poweroff.c
2. Just modify mt6323-poweroff.c file to compatible this patch. I mean
using RTC_WRTGR_MT6397 to replace RTC_WRTGR. Or modify mt6323-poweroff.c
like rtc-mt6397.c
Nicolas Boichat March 4, 2020, 12:59 p.m. UTC | #3
Hi,

On Mon, Feb 17, 2020 at 11:52 AM Ran Bi <ran.bi@mediatek.com> wrote:
>
> On Tue, 2020-02-04 at 00:50 +0800, Yingjoe Chen wrote:
> > > diff --git a/include/linux/mfd/mt6397/rtc.h b/include/linux/mfd/mt6397/rtc.h
> > > index f84b916..fffe34a 100644
> > > --- a/include/linux/mfd/mt6397/rtc.h
> > > +++ b/include/linux/mfd/mt6397/rtc.h
> > > @@ -18,7 +18,8 @@
> > >  #define RTC_BBPU_CBUSY         BIT(6)
> > >  #define RTC_BBPU_KEY            (0x43 << 8)
> > >
> > > -#define RTC_WRTGR              0x003c
> > > +#define RTC_WRTGR_MT6358       0x3a
> > > +#define RTC_WRTGR_MT6397       0x3c
> > >
> > >  #define RTC_IRQ_STA            0x0002
> > >  #define RTC_IRQ_STA_AL         BIT(0)
> > > @@ -57,6 +58,10 @@
> > >  #define MTK_RTC_POLL_DELAY_US  10
> > >  #define MTK_RTC_POLL_TIMEOUT   (jiffies_to_usecs(HZ))
> > >
> > > +struct mtk_rtc_data {
> > > +   u32                     wrtgr;
> > > +};
> > > +
> > >  struct mt6397_rtc {
> > >     struct device           *dev;
> > >     struct rtc_device       *rtc_dev;
> > > @@ -66,6 +71,15 @@ struct mt6397_rtc {
> > >     struct regmap           *regmap;
> > >     int                     irq;
> > >     u32                     addr_base;
> > > +   const struct mtk_rtc_data *data;
> > > +};
> > > +
> > > +static const struct mtk_rtc_data mt6358_rtc_data = {
> > > +   .wrtgr = RTC_WRTGR_MT6358,
> > > +};
> > > +
> > > +static const struct mtk_rtc_data mt6397_rtc_data = {
> > > +   .wrtgr = RTC_WRTGR_MT6397,
> > >  };
> >
> > Hi,
> >
> > Putting these in header file doesn't looks right to me.
> > Who need this? can you move them back to rtc-mt6397.c?
> > Joe.C
> >
>
> This could also effect kernel/drivers/power/reset/mt6323-poweroff.c
> which using same region of RTC registers.
> There are 2 ways of modification:
> 1. kernel/drivers/rtc/rtc-mt6397.c implement do_pwroff function and
> export to mt6323-poweroff.c
> 2. Just modify mt6323-poweroff.c file to compatible this patch. I mean
> using RTC_WRTGR_MT6397 to replace RTC_WRTGR. Or modify mt6323-poweroff.c
> like rtc-mt6397.c

Oh, I see, so basically both rtc-mt6397.c and mt6323-poweroff.c need
to know at what offset RTC_WRTGR actually is. Correct?

Is there any plan to have mt6323-poweroff.c support any of the other
PMICs (not just MT6323?)?

a. If not, I'd just add:
#define RTC_WRTGR_MT6323 RTC_WRTGR_MT6397
in rtc.h, for added clarity, use that in mt6323-poweroff.c
(s/RTC_WRTGR/RTC_WRTGR_MT6323/), and be done with it.

Actually, even if there's a plan, you can go ahead with this simpler
solution for now, and fix later when the issue comes up.

b. If you ever want to support multiple PMICs with mt6323-poweroff.c,
you'd need that offset for 2 different sub-devices under the same mfd,
so the matching logic belongs in the main mfd device, not in
rtc/poweroff driver.

So I'd move the matching logic in drivers/mfd/mt6397-core.c, and add
rtc_wrtgr offset (or a full _data structure) to `struct mt6397_chip`,
or, probably better, add a IORESOURCE_REG to the matching resources to
specify the offset (that's what drivers/mfd/88pm860x-core.c seems to
be doing, for example).

And then mt6323-poweroff.c should probably be renamed to mt6397-poweroff.c.

(actually, looking at this, I'm even questioning if mt6323-poweroff.c
should even exist, and if you should just fold it into rtc-mt6397.c?
Since they use the same registers?)

Hope this makes sense?

Best,
Ran Bi March 5, 2020, 7:37 a.m. UTC | #4
Hi,

On Wed, 2020-03-04 at 20:59 +0800, Nicolas Boichat wrote:
> Hi,
> 
> On Mon, Feb 17, 2020 at 11:52 AM Ran Bi <ran.bi@mediatek.com> wrote:
> >
> > On Tue, 2020-02-04 at 00:50 +0800, Yingjoe Chen wrote:
> > > > diff --git a/include/linux/mfd/mt6397/rtc.h b/include/linux/mfd/mt6397/rtc.h
> > > > index f84b916..fffe34a 100644
> > > > --- a/include/linux/mfd/mt6397/rtc.h
> > > > +++ b/include/linux/mfd/mt6397/rtc.h
> > > > @@ -18,7 +18,8 @@
> > > >  #define RTC_BBPU_CBUSY         BIT(6)
> > > >  #define RTC_BBPU_KEY            (0x43 << 8)
> > > >
> > > > -#define RTC_WRTGR              0x003c
> > > > +#define RTC_WRTGR_MT6358       0x3a
> > > > +#define RTC_WRTGR_MT6397       0x3c
> > > >
> > > >  #define RTC_IRQ_STA            0x0002
> > > >  #define RTC_IRQ_STA_AL         BIT(0)
> > > > @@ -57,6 +58,10 @@
> > > >  #define MTK_RTC_POLL_DELAY_US  10
> > > >  #define MTK_RTC_POLL_TIMEOUT   (jiffies_to_usecs(HZ))
> > > >
> > > > +struct mtk_rtc_data {
> > > > +   u32                     wrtgr;
> > > > +};
> > > > +
> > > >  struct mt6397_rtc {
> > > >     struct device           *dev;
> > > >     struct rtc_device       *rtc_dev;
> > > > @@ -66,6 +71,15 @@ struct mt6397_rtc {
> > > >     struct regmap           *regmap;
> > > >     int                     irq;
> > > >     u32                     addr_base;
> > > > +   const struct mtk_rtc_data *data;
> > > > +};
> > > > +
> > > > +static const struct mtk_rtc_data mt6358_rtc_data = {
> > > > +   .wrtgr = RTC_WRTGR_MT6358,
> > > > +};
> > > > +
> > > > +static const struct mtk_rtc_data mt6397_rtc_data = {
> > > > +   .wrtgr = RTC_WRTGR_MT6397,
> > > >  };
> > >
> > > Hi,
> > >
> > > Putting these in header file doesn't looks right to me.
> > > Who need this? can you move them back to rtc-mt6397.c?
> > > Joe.C
> > >
> >
> > This could also effect kernel/drivers/power/reset/mt6323-poweroff.c
> > which using same region of RTC registers.
> > There are 2 ways of modification:
> > 1. kernel/drivers/rtc/rtc-mt6397.c implement do_pwroff function and
> > export to mt6323-poweroff.c
> > 2. Just modify mt6323-poweroff.c file to compatible this patch. I mean
> > using RTC_WRTGR_MT6397 to replace RTC_WRTGR. Or modify mt6323-poweroff.c
> > like rtc-mt6397.c
> 
> Oh, I see, so basically both rtc-mt6397.c and mt6323-poweroff.c need
> to know at what offset RTC_WRTGR actually is. Correct?
> 

Yes, you are right both drivers need to know RTC_WRTGR offset. Offsets
of other registers are the same.

> Is there any plan to have mt6323-poweroff.c support any of the other
> PMICs (not just MT6323?)?
> 

Currently, we don't have a plan to let mt6323-poweroff.c support other
PMICs. Because other PMICs like mt6397 and mt6358 could using
arm-trust-firmware PSCI power off flow instead. mt6323-poweroff.c was
prepared for platform without arm-trust-firmware.

> a. If not, I'd just add:
> #define RTC_WRTGR_MT6323 RTC_WRTGR_MT6397
> in rtc.h, for added clarity, use that in mt6323-poweroff.c
> (s/RTC_WRTGR/RTC_WRTGR_MT6323/), and be done with it.
> 

I would just change RTC_WRTGR to RTC_WRTGR_MT6397 in mt6323-poweroff.c
at next patchset.

> Actually, even if there's a plan, you can go ahead with this simpler
> solution for now, and fix later when the issue comes up.
> 
> b. If you ever want to support multiple PMICs with mt6323-poweroff.c,
> you'd need that offset for 2 different sub-devices under the same mfd,
> so the matching logic belongs in the main mfd device, not in
> rtc/poweroff driver.
> 
> So I'd move the matching logic in drivers/mfd/mt6397-core.c, and add
> rtc_wrtgr offset (or a full _data structure) to `struct mt6397_chip`,
> or, probably better, add a IORESOURCE_REG to the matching resources to
> specify the offset (that's what drivers/mfd/88pm860x-core.c seems to
> be doing, for example).
> 
> And then mt6323-poweroff.c should probably be renamed to mt6397-poweroff.c.
> 
> (actually, looking at this, I'm even questioning if mt6323-poweroff.c
> should even exist, and if you should just fold it into rtc-mt6397.c?
> Since they use the same registers?)
> 

mt6323-poweroff.c which hijack pm_power_off pointer is only for platform
without arm-trust-firmware. This is the reason I am considering
mt6323-poweroff.c should not be folded into rtc-mt6397.c.

> Hope this makes sense?
> 
> Best,

Thanks for your suggestions.

Best,
Yingjoe Chen March 9, 2020, 2:33 a.m. UTC | #5
On Thu, 2020-03-05 at 15:37 +0800, Ran Bi wrote:
> Hi,
> 
> On Wed, 2020-03-04 at 20:59 +0800, Nicolas Boichat wrote:
> > Hi,
> > 
> > On Mon, Feb 17, 2020 at 11:52 AM Ran Bi <ran.bi@mediatek.com> wrote:
> > >
> > > On Tue, 2020-02-04 at 00:50 +0800, Yingjoe Chen wrote:

<....>

> > > >
> > > > Putting these in header file doesn't looks right to me.
> > > > Who need this? can you move them back to rtc-mt6397.c?
> > > > Joe.C
> > > >
> > >
> > > This could also effect kernel/drivers/power/reset/mt6323-poweroff.c
> > > which using same region of RTC registers.
> > > There are 2 ways of modification:
> > > 1. kernel/drivers/rtc/rtc-mt6397.c implement do_pwroff function and
> > > export to mt6323-poweroff.c
> > > 2. Just modify mt6323-poweroff.c file to compatible this patch. I mean
> > > using RTC_WRTGR_MT6397 to replace RTC_WRTGR. Or modify mt6323-poweroff.c
> > > like rtc-mt6397.c
> > 
> > Oh, I see, so basically both rtc-mt6397.c and mt6323-poweroff.c need
> > to know at what offset RTC_WRTGR actually is. Correct?
> > 
> 
> Yes, you are right both drivers need to know RTC_WRTGR offset. Offsets
> of other registers are the same.
> 
> > Is there any plan to have mt6323-poweroff.c support any of the other
> > PMICs (not just MT6323?)?
> > 
> 
> Currently, we don't have a plan to let mt6323-poweroff.c support other
> PMICs. Because other PMICs like mt6397 and mt6358 could using
> arm-trust-firmware PSCI power off flow instead. mt6323-poweroff.c was
> prepared for platform without arm-trust-firmware.

This depends on SoC instead of PMIC.
We will need mt6323-poweroff.c for soc with armv7 CPU, because we won't
have ATF on them. I'm not aware of new plan for this.


> > a. If not, I'd just add:
> > #define RTC_WRTGR_MT6323 RTC_WRTGR_MT6397
> > in rtc.h, for added clarity, use that in mt6323-poweroff.c
> > (s/RTC_WRTGR/RTC_WRTGR_MT6323/), and be done with it.
> > 
> 
> I would just change RTC_WRTGR to RTC_WRTGR_MT6397 in mt6323-poweroff.c
> at next patchset.
> 
> > Actually, even if there's a plan, you can go ahead with this simpler
> > solution for now, and fix later when the issue comes up.
> > 
> > b. If you ever want to support multiple PMICs with mt6323-poweroff.c,
> > you'd need that offset for 2 different sub-devices under the same mfd,
> > so the matching logic belongs in the main mfd device, not in
> > rtc/poweroff driver.
> > 
> > So I'd move the matching logic in drivers/mfd/mt6397-core.c, and add
> > rtc_wrtgr offset (or a full _data structure) to `struct mt6397_chip`,
> > or, probably better, add a IORESOURCE_REG to the matching resources to
> > specify the offset (that's what drivers/mfd/88pm860x-core.c seems to
> > be doing, for example).
> > 
> > And then mt6323-poweroff.c should probably be renamed to mt6397-poweroff.c.
> > 
> > (actually, looking at this, I'm even questioning if mt6323-poweroff.c
> > should even exist, and if you should just fold it into rtc-mt6397.c?
> > Since they use the same registers?)
> > 
> 
> mt6323-poweroff.c which hijack pm_power_off pointer is only for platform
> without arm-trust-firmware. This is the reason I am considering
> mt6323-poweroff.c should not be folded into rtc-mt6397.c.


Using/sharing same set of registers from different drivers is not good: 

- WRTGR is a special register to 'commit' previous changes. If 2 drivers
are running at the same time, it is possible to commit incomplete update
and cause unexpected result. It is easier to control this from same
driver.

- It is easy to overlook the register is access by others and lead to
bugs/build fails when doing driver update, eg, this patchset.

- The trigger code is duplicate in mt6323-poweroff.c, can just call
mtk_rtc_write_trigger.


So I agree with Nicolas, mt6323-poweroff should be folded into
rtc-mt6397.c. We should be able to disable pm_power_off hijacking for
platform with armV8 CPU. Maybe we can keep "mediatek,mt6323-pwrc"
compatible in mt6323-poweroff.c for this.

I'm ok with implement a. as suggested by Nicolas for now.

Joe.C

Patch
diff mbox series

diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
index 5249fc9..a90735e1 100644
--- a/drivers/rtc/rtc-mt6397.c
+++ b/drivers/rtc/rtc-mt6397.c
@@ -9,18 +9,31 @@ 
 #include <linux/mfd/mt6397/core.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
+#include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
 #include <linux/rtc.h>
 #include <linux/mfd/mt6397/rtc.h>
 #include <linux/mod_devicetable.h>
 
+static const struct of_device_id mt6397_rtc_of_match[] = {
+	{ .compatible = "mediatek,mt6323-rtc",
+		.data = (void *)&mt6397_rtc_data, },
+	{ .compatible = "mediatek,mt6358-rtc",
+		.data = (void *)&mt6358_rtc_data, },
+	{ .compatible = "mediatek,mt6397-rtc",
+		.data = (void *)&mt6397_rtc_data, },
+	{}
+};
+MODULE_DEVICE_TABLE(of, mt6397_rtc_of_match);
+
 static int mtk_rtc_write_trigger(struct mt6397_rtc *rtc)
 {
 	int ret;
 	u32 data;
 
-	ret = regmap_write(rtc->regmap, rtc->addr_base + RTC_WRTGR, 1);
+	ret = regmap_write(rtc->regmap,
+			   rtc->addr_base + rtc->data->wrtgr, 1);
 	if (ret < 0)
 		return ret;
 
@@ -258,6 +271,9 @@  static int mtk_rtc_probe(struct platform_device *pdev)
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	rtc->addr_base = res->start;
 
+	rtc->data = (struct mtk_rtc_data *)
+			of_device_get_match_data(&pdev->dev);
+
 	rtc->irq = platform_get_irq(pdev, 0);
 	if (rtc->irq < 0)
 		return rtc->irq;
@@ -322,13 +338,6 @@  static int mt6397_rtc_resume(struct device *dev)
 static SIMPLE_DEV_PM_OPS(mt6397_pm_ops, mt6397_rtc_suspend,
 			mt6397_rtc_resume);
 
-static const struct of_device_id mt6397_rtc_of_match[] = {
-	{ .compatible = "mediatek,mt6323-rtc", },
-	{ .compatible = "mediatek,mt6397-rtc", },
-	{ }
-};
-MODULE_DEVICE_TABLE(of, mt6397_rtc_of_match);
-
 static struct platform_driver mtk_rtc_driver = {
 	.driver = {
 		.name = "mt6397-rtc",
diff --git a/include/linux/mfd/mt6397/rtc.h b/include/linux/mfd/mt6397/rtc.h
index f84b916..fffe34a 100644
--- a/include/linux/mfd/mt6397/rtc.h
+++ b/include/linux/mfd/mt6397/rtc.h
@@ -18,7 +18,8 @@ 
 #define RTC_BBPU_CBUSY         BIT(6)
 #define RTC_BBPU_KEY            (0x43 << 8)
 
-#define RTC_WRTGR              0x003c
+#define RTC_WRTGR_MT6358       0x3a
+#define RTC_WRTGR_MT6397       0x3c
 
 #define RTC_IRQ_STA            0x0002
 #define RTC_IRQ_STA_AL         BIT(0)
@@ -57,6 +58,10 @@ 
 #define MTK_RTC_POLL_DELAY_US  10
 #define MTK_RTC_POLL_TIMEOUT   (jiffies_to_usecs(HZ))
 
+struct mtk_rtc_data {
+	u32			wrtgr;
+};
+
 struct mt6397_rtc {
 	struct device           *dev;
 	struct rtc_device       *rtc_dev;
@@ -66,6 +71,15 @@  struct mt6397_rtc {
 	struct regmap           *regmap;
 	int                     irq;
 	u32                     addr_base;
+	const struct mtk_rtc_data *data;
+};
+
+static const struct mtk_rtc_data mt6358_rtc_data = {
+	.wrtgr = RTC_WRTGR_MT6358,
+};
+
+static const struct mtk_rtc_data mt6397_rtc_data = {
+	.wrtgr = RTC_WRTGR_MT6397,
 };
 
 #endif /* _LINUX_MFD_MT6397_RTC_H_ */