Re: [PATCH v2 2/4] rtc: Add support for the MediaTek MT2712 RTC
On Thu, 2019-08-22 at 15:36 +0200, Alexandre Belloni wrote: > On 22/08/2019 21:26:01+0800, Ran Bi wrote: > > On Thu, 2019-08-22 at 14:46 +0200, Alexandre Belloni wrote: > > > On 22/08/2019 20:34:14+0800, Ran Bi wrote: > > > > > > + /* RTC need POWERKEY1/2 match, then goto normal work mode */ > > > > > > + mt2712_writel(rtc, MT2712_POWERKEY1, MT2712_POWERKEY1_KEY); > > > > > > + mt2712_writel(rtc, MT2712_POWERKEY2, MT2712_POWERKEY2_KEY); > > > > > > > > > > This should be written when setting the time after power was lost. > > > > > > > > > > > > > I suppose we can move this into mt2712_rtc_read_time function's "if > > > > (p1 != MT2712_POWERKEY1_KEY || p2 != MT2712_POWERKEY2_KEY)" condition > > > > which will be added at next patch. We need additional flag to mark this > > > > condition or another if condition in mt2712_rtc_set_time fucntion if we > > > > put these code in mt2712_rtc_set_time function. > > > > > > > > > > It is fine to test both in read_time and in set_time. > > > > > > > Do you mean that we can test powerkey and then set powerkey both in > > read_time and in set_time? > > > > I mean that can test in read_time and test and set in set_time > > Ok, I will change it at next patch. Best Regards, Ran
Re: [PATCH v2 2/4] rtc: Add support for the MediaTek MT2712 RTC
On 22/08/2019 21:26:01+0800, Ran Bi wrote: > On Thu, 2019-08-22 at 14:46 +0200, Alexandre Belloni wrote: > > On 22/08/2019 20:34:14+0800, Ran Bi wrote: > > > > > + /* RTC need POWERKEY1/2 match, then goto normal work mode */ > > > > > + mt2712_writel(rtc, MT2712_POWERKEY1, MT2712_POWERKEY1_KEY); > > > > > + mt2712_writel(rtc, MT2712_POWERKEY2, MT2712_POWERKEY2_KEY); > > > > > > > > This should be written when setting the time after power was lost. > > > > > > > > > > I suppose we can move this into mt2712_rtc_read_time function's "if > > > (p1 != MT2712_POWERKEY1_KEY || p2 != MT2712_POWERKEY2_KEY)" condition > > > which will be added at next patch. We need additional flag to mark this > > > condition or another if condition in mt2712_rtc_set_time fucntion if we > > > put these code in mt2712_rtc_set_time function. > > > > > > > It is fine to test both in read_time and in set_time. > > > > Do you mean that we can test powerkey and then set powerkey both in > read_time and in set_time? > I mean that can test in read_time and test and set in set_time -- Alexandre Belloni, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH v2 2/4] rtc: Add support for the MediaTek MT2712 RTC
On Thu, 2019-08-22 at 14:46 +0200, Alexandre Belloni wrote: > On 22/08/2019 20:34:14+0800, Ran Bi wrote: > > > > + /* RTC need POWERKEY1/2 match, then goto normal work mode */ > > > > + mt2712_writel(rtc, MT2712_POWERKEY1, MT2712_POWERKEY1_KEY); > > > > + mt2712_writel(rtc, MT2712_POWERKEY2, MT2712_POWERKEY2_KEY); > > > > > > This should be written when setting the time after power was lost. > > > > > > > I suppose we can move this into mt2712_rtc_read_time function's "if > > (p1 != MT2712_POWERKEY1_KEY || p2 != MT2712_POWERKEY2_KEY)" condition > > which will be added at next patch. We need additional flag to mark this > > condition or another if condition in mt2712_rtc_set_time fucntion if we > > put these code in mt2712_rtc_set_time function. > > > > It is fine to test both in read_time and in set_time. > Do you mean that we can test powerkey and then set powerkey both in read_time and in set_time?
Re: [PATCH v2 2/4] rtc: Add support for the MediaTek MT2712 RTC
On 22/08/2019 20:34:14+0800, Ran Bi wrote: > > > + /* RTC need POWERKEY1/2 match, then goto normal work mode */ > > > + mt2712_writel(rtc, MT2712_POWERKEY1, MT2712_POWERKEY1_KEY); > > > + mt2712_writel(rtc, MT2712_POWERKEY2, MT2712_POWERKEY2_KEY); > > > > This should be written when setting the time after power was lost. > > > > I suppose we can move this into mt2712_rtc_read_time function's "if > (p1 != MT2712_POWERKEY1_KEY || p2 != MT2712_POWERKEY2_KEY)" condition > which will be added at next patch. We need additional flag to mark this > condition or another if condition in mt2712_rtc_set_time fucntion if we > put these code in mt2712_rtc_set_time function. > It is fine to test both in read_time and in set_time. -- Alexandre Belloni, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH v2 2/4] rtc: Add support for the MediaTek MT2712 RTC
Hi,
> > +
> > +#define MTK_RTC_DEVKBUILD_MODNAME
>
> You probably shouldn't do that and have a static string for the driver
> name. I probably doesn't matter much though because DT is used to probe
> the driver.
>
Will change it at next patch.
> > +/* we map HW YEAR 0 to 2000 because 2000 is the leap year */
> > +#define MT2712_MIN_YEAR2000
> > +#define MT2712_BASE_YEAR 1900
> > +#define MT2712_MIN_YEAR_OFFSET (MT2712_MIN_YEAR - MT2712_BASE_YEAR)
> > +#define MT2712_MAX_YEAR_OFFSET (MT2712_MIN_YEAR_OFFSET + 127)
> > +
>
> All those defines are unecessary, see below.
>
Will change it at next patch.
> > +struct mt2712_rtc {
> > + struct device *dev;
>
> Looking at the code closely, it seems this is only used for debug and
> error messages. Maybe you could use rtc_dev->dev instead.
>
Will change it at next patch.
> > + mutex_lock(>rtc_dev->ops_lock);
> > +
> > + irqsta = mt2712_readl(rtc, MT2712_IRQ_STA);
>
> Do you have to lock that read? Is the register cleared on read?
>
Yes, this register is read clear register.
> > + do {
> > + __mt2712_rtc_read_time(rtc, tm, );
> > + } while (sec < tm->tm_sec); /* SEC has carried */
>
> Shouldn't that be while (tm->tm_sec < sec)?
>
In __mt2712_rtc_read_time function, we read tm->tm_sec before read sec.
Sometimes we can meet situation like "tm->tm_sec == 59" and "sec == 0".
It means that TC_SEC has carried and we need to reload the tm struct. I
suppose it was correct that using "while (sec < tm->tm_sec)"
> > +
> > + /* HW register use 7 bits to store year data, minus
> > +* MT2712_MIN_YEAR_OFFSET brfore write year data to register, and plus
> > +* MT2712_MIN_YEAR_OFFSET back after read year from register
> > +*/
> > + tm->tm_year += MT2712_MIN_YEAR_OFFSET;
>
> Simply add 100 in __mt2712_rtc_read_time
>
Will change it at next patch.
> > +
> > + /* HW register start mon from one, but tm_mon start from zero. */
> > + tm->tm_mon--;
> > +
>
> You can also do that in __mt2712_rtc_read_time.
>
Will change it at next patch.
> > + if (rtc_valid_tm(tm)) {
>
> This check is unnecessary, the validity is always checked by the core.
>
Will remove this at next patch.
> > + if (tm->tm_year > MT2712_MAX_YEAR_OFFSET) {
> > + dev_dbg(rtc->dev, "Set year %d out of range. (%d - %d)\n",
> > + 1900 + tm->tm_year, 1900 + MT2712_MIN_YEAR_OFFSET,
> > + 1900 + MT2712_MAX_YEAR_OFFSET);
> > + return -EINVAL;
> > + }
>
> This check is unnecessary, see below.
>
Will change it at next patch.
> > +
> > + tm->tm_year -= MT2712_MIN_YEAR_OFFSET;
> > + tm->tm_mon++;
>
> You should probably avoid modifying tm, move the substraction and
> addition in the mt2712_writel calls.
>
Will change it at next patch.
> > + if (tm->tm_year > MT2712_MAX_YEAR_OFFSET) {
> > + dev_dbg(rtc->dev, "Set year %d out of range. (%d - %d)\n",
> > + 1900 + tm->tm_year, 1900 + MT2712_MIN_YEAR_OFFSET,
> > + 1900 + MT2712_MAX_YEAR_OFFSET);
> > + return -EINVAL;
> > + }
> > +
>
> Unnecessary check.
>
Will change it at next patch.
> > + p1 = mt2712_readl(rtc, MT2712_POWERKEY1);
> > + p2 = mt2712_readl(rtc, MT2712_POWERKEY2);
> > + if (p1 != MT2712_POWERKEY1_KEY || p2 != MT2712_POWERKEY2_KEY)
> > + dev_dbg(rtc->dev, "powerkey not set (lost power)\n");
> > +
>
> This info is valuable, you should check that when reading the time and
> return -EINVAL if power was lost.
>
Will change it at next patch.
>
> > + /* RTC need POWERKEY1/2 match, then goto normal work mode */
> > + mt2712_writel(rtc, MT2712_POWERKEY1, MT2712_POWERKEY1_KEY);
> > + mt2712_writel(rtc, MT2712_POWERKEY2, MT2712_POWERKEY2_KEY);
>
> This should be written when setting the time after power was lost.
>
I suppose we can move this into mt2712_rtc_read_time function's "if
(p1 != MT2712_POWERKEY1_KEY || p2 != MT2712_POWERKEY2_KEY)" condition
which will be added at next patch. We need additional flag to mark this
condition or another if condition in mt2712_rtc_set_time fucntion if we
put these code in mt2712_rtc_set_time function.
> > +static const struct rtc_class_ops mt2712_rtc_ops = {
> > + .read_time = mt2712_rtc_read_time,
> > + .set_time = mt2712_rtc_set_time,
> > + .read_alarm = mt2712_rtc_read_alarm,
> > + .set_alarm = mt2712_rtc_set_alarm,
>
> For proper operations, you should also provide the .alarm_irq_enable
> callback.
>
Will change it at next patch.
> > + rtc->rtc_dev->ops = _rtc_ops;
>
> If you set the range properly here using rtc_dev->range_min and
> rtc_dev->range_max, then the core will be able to do range checking and
> will also take care of the year offset/windowing calculations instead of
> having to hardcode that in the driver.
>
Will change it at next patch.
Best Regards,
Ran
Re: [PATCH v2 2/4] rtc: Add support for the MediaTek MT2712 RTC
On Thu, 2019-08-22 at 11:20 +0200, Alexandre Belloni wrote: > On 22/08/2019 11:12:29+0200, Matthias Brugger wrote: > > > > > > On 01/08/2019 13:01, Ran Bi wrote: > > > This add support for the MediaTek MT2712 RTC. It was SoC based RTC, but > > > had different architecture compared with MT7622 RTC. > > > > > > Signed-off-by: Ran Bi > > > --- > > > drivers/rtc/Kconfig | 10 + > > > drivers/rtc/Makefile | 1 + > > > drivers/rtc/rtc-mt2712.c | 444 +++ > > > > Can't we just adjust rtc-mt7622.c (and rename it) to unify the source for > > both > > devices. What is the difference that we need to write a driver of our own? > > > > If they are compatible, this is the way to go but the file can't be > renamed (and that is fine). > > They are not compatible. Both registers and operating methods are different. Best Regards, Ran
Re: [PATCH v2 2/4] rtc: Add support for the MediaTek MT2712 RTC
On Thu, 2019-08-22 at 11:12 +0200, Matthias Brugger wrote: > > On 01/08/2019 13:01, Ran Bi wrote: > > This add support for the MediaTek MT2712 RTC. It was SoC based RTC, but > > had different architecture compared with MT7622 RTC. > > > > Signed-off-by: Ran Bi > > --- > > drivers/rtc/Kconfig | 10 + > > drivers/rtc/Makefile | 1 + > > drivers/rtc/rtc-mt2712.c | 444 +++ > > Can't we just adjust rtc-mt7622.c (and rename it) to unify the source for both > devices. What is the difference that we need to write a driver of our own? > > Regards, > Matthias We cannot merge rtc-mt7622.c and rtc-mt2712.c together. These two rtc hardwares have totally different design. Registers naming, registers offset and operating method are different.
Re: [PATCH v2 2/4] rtc: Add support for the MediaTek MT2712 RTC
On 22/08/2019 11:12:29+0200, Matthias Brugger wrote: > > > On 01/08/2019 13:01, Ran Bi wrote: > > This add support for the MediaTek MT2712 RTC. It was SoC based RTC, but > > had different architecture compared with MT7622 RTC. > > > > Signed-off-by: Ran Bi > > --- > > drivers/rtc/Kconfig | 10 + > > drivers/rtc/Makefile | 1 + > > drivers/rtc/rtc-mt2712.c | 444 +++ > > Can't we just adjust rtc-mt7622.c (and rename it) to unify the source for both > devices. What is the difference that we need to write a driver of our own? > If they are compatible, this is the way to go but the file can't be renamed (and that is fine). -- Alexandre Belloni, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH v2 2/4] rtc: Add support for the MediaTek MT2712 RTC
On 01/08/2019 13:01, Ran Bi wrote:
> This add support for the MediaTek MT2712 RTC. It was SoC based RTC, but
> had different architecture compared with MT7622 RTC.
>
> Signed-off-by: Ran Bi
> ---
> drivers/rtc/Kconfig | 10 +
> drivers/rtc/Makefile | 1 +
> drivers/rtc/rtc-mt2712.c | 444 +++
Can't we just adjust rtc-mt7622.c (and rename it) to unify the source for both
devices. What is the difference that we need to write a driver of our own?
Regards,
Matthias
> 3 files changed, 455 insertions(+)
> create mode 100644 drivers/rtc/rtc-mt2712.c
>
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> index e72f65b61176..977d0f480dc7 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -1772,6 +1772,16 @@ config RTC_DRV_MOXART
> This driver can also be built as a module. If so, the module
> will be called rtc-moxart
>
> +config RTC_DRV_MT2712
> + tristate "MediaTek MT2712 SoC based RTC"
> + depends on ARCH_MEDIATEK || COMPILE_TEST
> + help
> + This enables support for the real time clock built in the MediaTek
> + SoCs for MT2712.
> +
> + This drive can also be built as a module. If so, the module
> + will be called rtc-mt2712.
> +
> config RTC_DRV_MT6397
> tristate "MediaTek PMIC based RTC"
> depends on MFD_MT6397 || (COMPILE_TEST && IRQ_DOMAIN)
> diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
> index 6b09c21dc1b6..7c6cf70af281 100644
> --- a/drivers/rtc/Makefile
> +++ b/drivers/rtc/Makefile
> @@ -108,6 +108,7 @@ obj-$(CONFIG_RTC_DRV_MESON) += rtc-meson.o
> obj-$(CONFIG_RTC_DRV_MOXART) += rtc-moxart.o
> obj-$(CONFIG_RTC_DRV_MPC5121)+= rtc-mpc5121.o
> obj-$(CONFIG_RTC_DRV_MSM6242)+= rtc-msm6242.o
> +obj-$(CONFIG_RTC_DRV_MT2712) += rtc-mt2712.o
> obj-$(CONFIG_RTC_DRV_MT6397) += rtc-mt6397.o
> obj-$(CONFIG_RTC_DRV_MT7622) += rtc-mt7622.o
> obj-$(CONFIG_RTC_DRV_MV) += rtc-mv.o
> diff --git a/drivers/rtc/rtc-mt2712.c b/drivers/rtc/rtc-mt2712.c
> new file mode 100644
> index ..1eb71ca64c2c
> --- /dev/null
> +++ b/drivers/rtc/rtc-mt2712.c
> @@ -0,0 +1,444 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2019 MediaTek Inc.
> + * Author: Ran Bi
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#define MTK_RTC_DEV KBUILD_MODNAME
> +
> +#define MT2712_BBPU 0x
> +#define MT2712_BBPU_CLRPKY BIT(4)
> +#define MT2712_BBPU_RELOAD BIT(5)
> +#define MT2712_BBPU_CBUSYBIT(6)
> +#define MT2712_BBPU_KEY (0x43 << 8)
> +
> +#define MT2712_IRQ_STA 0x0004
> +#define MT2712_IRQ_STA_ALBIT(0)
> +#define MT2712_IRQ_STA_TCBIT(1)
> +
> +#define MT2712_IRQ_EN0x0008
> +#define MT2712_IRQ_EN_AL BIT(0)
> +#define MT2712_IRQ_EN_TC BIT(1)
> +#define MT2712_IRQ_EN_ONESHOTBIT(2)
> +#define MT2712_IRQ_EN_ONESHOT_AL \
> + (MT2712_IRQ_EN_ONESHOT | MT2712_IRQ_EN_AL)
> +
> +#define MT2712_CII_EN0x000c
> +
> +#define MT2712_AL_MASK 0x0010
> +#define MT2712_AL_MASK_DOW BIT(4)
> +
> +#define MT2712_TC_SEC0x0014
> +#define MT2712_TC_MIN0x0018
> +#define MT2712_TC_HOU0x001c
> +#define MT2712_TC_DOM0x0020
> +#define MT2712_TC_DOW0x0024
> +#define MT2712_TC_MTH0x0028
> +#define MT2712_TC_YEA0x002c
> +
> +#define MT2712_AL_SEC0x0030
> +#define MT2712_AL_MIN0x0034
> +#define MT2712_AL_HOU0x0038
> +#define MT2712_AL_DOM0x003c
> +#define MT2712_AL_DOW0x0040
> +#define MT2712_AL_MTH0x0044
> +#define MT2712_AL_YEA0x0048
> +
> +#define MT2712_SEC_MASK 0x003f
> +#define MT2712_MIN_MASK 0x003f
> +#define MT2712_HOU_MASK 0x001f
> +#define MT2712_DOM_MASK 0x001f
> +#define MT2712_DOW_MASK 0x0007
> +#define MT2712_MTH_MASK 0x000f
> +#define MT2712_YEA_MASK 0x007f
> +
> +#define MT2712_POWERKEY1 0x004c
> +#define MT2712_POWERKEY2 0x0050
> +#define MT2712_POWERKEY1_KEY 0xa357
> +#define MT2712_POWERKEY2_KEY 0x67d2
> +
> +#define MT2712_CON0 0x005c
> +#define MT2712_CON1 0x0060
> +
> +#define MT2712_PROT 0x0070
> +#define MT2712_PROT_UNLOCK1 0x9136
> +#define MT2712_PROT_UNLOCK2 0x586a
> +
> +#define MT2712_WRTGR 0x0078
> +
> +/* we map HW YEAR 0 to 2000 because 2000 is the leap year */
> +#define MT2712_MIN_YEAR 2000
> +#define MT2712_BASE_YEAR 1900
> +#define MT2712_MIN_YEAR_OFFSET (MT2712_MIN_YEAR - MT2712_BASE_YEAR)
> +#define MT2712_MAX_YEAR_OFFSET (MT2712_MIN_YEAR_OFFSET + 127)
> +
> +struct mt2712_rtc {
> + struct device
Re: [PATCH v2 2/4] rtc: Add support for the MediaTek MT2712 RTC
Hi,
On 01/08/2019 19:01:20+0800, Ran Bi wrote:
> diff --git a/drivers/rtc/rtc-mt2712.c b/drivers/rtc/rtc-mt2712.c
> new file mode 100644
> index ..1eb71ca64c2c
> --- /dev/null
> +++ b/drivers/rtc/rtc-mt2712.c
> @@ -0,0 +1,444 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2019 MediaTek Inc.
> + * Author: Ran Bi
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#define MTK_RTC_DEV KBUILD_MODNAME
You probably shouldn't do that and have a static string for the driver
name. I probably doesn't matter much though because DT is used to probe
the driver.
> +#define MT2712_WRTGR 0x0078
> +
> +/* we map HW YEAR 0 to 2000 because 2000 is the leap year */
> +#define MT2712_MIN_YEAR 2000
> +#define MT2712_BASE_YEAR 1900
> +#define MT2712_MIN_YEAR_OFFSET (MT2712_MIN_YEAR - MT2712_BASE_YEAR)
> +#define MT2712_MAX_YEAR_OFFSET (MT2712_MIN_YEAR_OFFSET + 127)
> +
All those defines are unecessary, see below.
> +struct mt2712_rtc {
> + struct device *dev;
Looking at the code closely, it seems this is only used for debug and
error messages. Maybe you could use rtc_dev->dev instead.
> + struct rtc_device *rtc_dev;
> + void __iomem*base;
> + int irq;
> + u8 irq_wake_enabled;
> +};
> +
> +static inline u32 mt2712_readl(struct mt2712_rtc *rtc, u32 reg)
> +{
> + return readl(rtc->base + reg);
> +}
> +
> +static inline void mt2712_writel(struct mt2712_rtc *rtc, u32 reg, u32 val)
> +{
> + writel(val, rtc->base + reg);
> +}
> +
> +static void mt2712_rtc_write_trigger(struct mt2712_rtc *rtc)
> +{
> + unsigned long timeout = jiffies + HZ/10;
> +
> + mt2712_writel(rtc, MT2712_WRTGR, 1);
> + while (1) {
> + if (!(mt2712_readl(rtc, MT2712_BBPU) & MT2712_BBPU_CBUSY))
> + break;
> +
> + if (time_after(jiffies, timeout)) {
> + dev_err(rtc->dev, "%s time out!\n", __func__);
> + break;
> + }
> + cpu_relax();
> + }
> +}
> +
> +static void mt2712_rtc_writeif_unlock(struct mt2712_rtc *rtc)
> +{
> + mt2712_writel(rtc, MT2712_PROT, MT2712_PROT_UNLOCK1);
> + mt2712_rtc_write_trigger(rtc);
> + mt2712_writel(rtc, MT2712_PROT, MT2712_PROT_UNLOCK2);
> + mt2712_rtc_write_trigger(rtc);
> +}
> +
> +static irqreturn_t rtc_irq_handler_thread(int irq, void *data)
> +{
> + struct mt2712_rtc *rtc = data;
> + u16 irqsta, irqen;
> +
> + mutex_lock(>rtc_dev->ops_lock);
> +
> + irqsta = mt2712_readl(rtc, MT2712_IRQ_STA);
Do you have to lock that read? Is the register cleared on read?
> + if (irqsta & MT2712_IRQ_STA_AL) {
> + rtc_update_irq(rtc->rtc_dev, 1, RTC_IRQF | RTC_AF);
> + irqen = irqsta & ~MT2712_IRQ_EN_AL;
> +
> + mt2712_writel(rtc, MT2712_IRQ_EN, irqen);
> + mt2712_rtc_write_trigger(rtc);
> +
> + mutex_unlock(>rtc_dev->ops_lock);
> + return IRQ_HANDLED;
> + }
> +
> + mutex_unlock(>rtc_dev->ops_lock);
> + return IRQ_NONE;
> +}
> +
> +static void __mt2712_rtc_read_time(struct mt2712_rtc *rtc,
> +struct rtc_time *tm, int *sec)
> +{
> + tm->tm_sec = mt2712_readl(rtc, MT2712_TC_SEC) & MT2712_SEC_MASK;
> + tm->tm_min = mt2712_readl(rtc, MT2712_TC_MIN) & MT2712_MIN_MASK;
> + tm->tm_hour = mt2712_readl(rtc, MT2712_TC_HOU) & MT2712_HOU_MASK;
> + tm->tm_mday = mt2712_readl(rtc, MT2712_TC_DOM) & MT2712_DOM_MASK;
> + tm->tm_mon = mt2712_readl(rtc, MT2712_TC_MTH) & MT2712_MTH_MASK;
> + tm->tm_year = mt2712_readl(rtc, MT2712_TC_YEA) & MT2712_YEA_MASK;
> +
> + *sec = mt2712_readl(rtc, MT2712_TC_SEC) & MT2712_SEC_MASK;
> +}
> +
> +static int mt2712_rtc_read_time(struct device *dev, struct rtc_time *tm)
> +{
> + struct mt2712_rtc *rtc = dev_get_drvdata(dev);
> + int sec;
> +
> + do {
> + __mt2712_rtc_read_time(rtc, tm, );
> + } while (sec < tm->tm_sec); /* SEC has carried */
Shouldn't that be while (tm->tm_sec < sec)?
> +
> + /* HW register use 7 bits to store year data, minus
> + * MT2712_MIN_YEAR_OFFSET brfore write year data to register, and plus
> + * MT2712_MIN_YEAR_OFFSET back after read year from register
> + */
> + tm->tm_year += MT2712_MIN_YEAR_OFFSET;
Simply add 100 in __mt2712_rtc_read_time
> +
> + /* HW register start mon from one, but tm_mon start from zero. */
> + tm->tm_mon--;
> +
You can also do that in __mt2712_rtc_read_time.
> + if (rtc_valid_tm(tm)) {
This check is unnecessary, the validity is always checked by the core.
> + dev_dbg(rtc->dev, "%s: invalid time %ptR\n", __func__, tm);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int mt2712_rtc_set_time(struct
[PATCH v2 2/4] rtc: Add support for the MediaTek MT2712 RTC
This add support for the MediaTek MT2712 RTC. It was SoC based RTC, but
had different architecture compared with MT7622 RTC.
Signed-off-by: Ran Bi
---
drivers/rtc/Kconfig | 10 +
drivers/rtc/Makefile | 1 +
drivers/rtc/rtc-mt2712.c | 444 +++
3 files changed, 455 insertions(+)
create mode 100644 drivers/rtc/rtc-mt2712.c
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index e72f65b61176..977d0f480dc7 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1772,6 +1772,16 @@ config RTC_DRV_MOXART
This driver can also be built as a module. If so, the module
will be called rtc-moxart
+config RTC_DRV_MT2712
+ tristate "MediaTek MT2712 SoC based RTC"
+ depends on ARCH_MEDIATEK || COMPILE_TEST
+ help
+ This enables support for the real time clock built in the MediaTek
+ SoCs for MT2712.
+
+ This drive can also be built as a module. If so, the module
+ will be called rtc-mt2712.
+
config RTC_DRV_MT6397
tristate "MediaTek PMIC based RTC"
depends on MFD_MT6397 || (COMPILE_TEST && IRQ_DOMAIN)
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 6b09c21dc1b6..7c6cf70af281 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -108,6 +108,7 @@ obj-$(CONFIG_RTC_DRV_MESON) += rtc-meson.o
obj-$(CONFIG_RTC_DRV_MOXART) += rtc-moxart.o
obj-$(CONFIG_RTC_DRV_MPC5121) += rtc-mpc5121.o
obj-$(CONFIG_RTC_DRV_MSM6242) += rtc-msm6242.o
+obj-$(CONFIG_RTC_DRV_MT2712) += rtc-mt2712.o
obj-$(CONFIG_RTC_DRV_MT6397) += rtc-mt6397.o
obj-$(CONFIG_RTC_DRV_MT7622) += rtc-mt7622.o
obj-$(CONFIG_RTC_DRV_MV) += rtc-mv.o
diff --git a/drivers/rtc/rtc-mt2712.c b/drivers/rtc/rtc-mt2712.c
new file mode 100644
index ..1eb71ca64c2c
--- /dev/null
+++ b/drivers/rtc/rtc-mt2712.c
@@ -0,0 +1,444 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2019 MediaTek Inc.
+ * Author: Ran Bi
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define MTK_RTC_DEVKBUILD_MODNAME
+
+#define MT2712_BBPU0x
+#define MT2712_BBPU_CLRPKY BIT(4)
+#define MT2712_BBPU_RELOAD BIT(5)
+#define MT2712_BBPU_CBUSY BIT(6)
+#define MT2712_BBPU_KEY(0x43 << 8)
+
+#define MT2712_IRQ_STA 0x0004
+#define MT2712_IRQ_STA_AL BIT(0)
+#define MT2712_IRQ_STA_TC BIT(1)
+
+#define MT2712_IRQ_EN 0x0008
+#define MT2712_IRQ_EN_AL BIT(0)
+#define MT2712_IRQ_EN_TC BIT(1)
+#define MT2712_IRQ_EN_ONESHOT BIT(2)
+#define MT2712_IRQ_EN_ONESHOT_AL \
+ (MT2712_IRQ_EN_ONESHOT | MT2712_IRQ_EN_AL)
+
+#define MT2712_CII_EN 0x000c
+
+#define MT2712_AL_MASK 0x0010
+#define MT2712_AL_MASK_DOW BIT(4)
+
+#define MT2712_TC_SEC 0x0014
+#define MT2712_TC_MIN 0x0018
+#define MT2712_TC_HOU 0x001c
+#define MT2712_TC_DOM 0x0020
+#define MT2712_TC_DOW 0x0024
+#define MT2712_TC_MTH 0x0028
+#define MT2712_TC_YEA 0x002c
+
+#define MT2712_AL_SEC 0x0030
+#define MT2712_AL_MIN 0x0034
+#define MT2712_AL_HOU 0x0038
+#define MT2712_AL_DOM 0x003c
+#define MT2712_AL_DOW 0x0040
+#define MT2712_AL_MTH 0x0044
+#define MT2712_AL_YEA 0x0048
+
+#define MT2712_SEC_MASK0x003f
+#define MT2712_MIN_MASK0x003f
+#define MT2712_HOU_MASK0x001f
+#define MT2712_DOM_MASK0x001f
+#define MT2712_DOW_MASK0x0007
+#define MT2712_MTH_MASK0x000f
+#define MT2712_YEA_MASK0x007f
+
+#define MT2712_POWERKEY1 0x004c
+#define MT2712_POWERKEY2 0x0050
+#define MT2712_POWERKEY1_KEY 0xa357
+#define MT2712_POWERKEY2_KEY 0x67d2
+
+#define MT2712_CON00x005c
+#define MT2712_CON10x0060
+
+#define MT2712_PROT0x0070
+#define MT2712_PROT_UNLOCK10x9136
+#define MT2712_PROT_UNLOCK20x586a
+
+#define MT2712_WRTGR 0x0078
+
+/* we map HW YEAR 0 to 2000 because 2000 is the leap year */
+#define MT2712_MIN_YEAR2000
+#define MT2712_BASE_YEAR 1900
+#define MT2712_MIN_YEAR_OFFSET (MT2712_MIN_YEAR - MT2712_BASE_YEAR)
+#define MT2712_MAX_YEAR_OFFSET (MT2712_MIN_YEAR_OFFSET + 127)
+
+struct mt2712_rtc {
+ struct device *dev;
+ struct rtc_device *rtc_dev;
+ void __iomem*base;
+ int irq;
+ u8 irq_wake_enabled;
+};
+
+static inline u32 mt2712_readl(struct mt2712_rtc *rtc, u32 reg)
+{
+ return readl(rtc->base + reg);
+}
+
+static inline void mt2712_writel(struct mt2712_rtc *rtc, u32 reg, u32 val)
+{
+ writel(val, rtc->base + reg);
+}
+
+static void mt2712_rtc_write_trigger(struct mt2712_rtc *rtc)
+{
+ unsigned long timeout = jiffies + HZ/10;
+
