Due to my unfamiliarity with the process of uploading patches to the
community, I accidentally triggered the upload of this incomplete patch
to the community.
I deeply apologize for wasting the time of all reviewers. I will
incorporate everyone's comments, make adjustments to my patch, and
upload a new version after internal review.
I apologize once again for my action.
Sincerely,
Shunxi Zhang
On Thu, 2024-10-31 at 16:58 +0100, Krzysztof Kozlowski wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>
>
> On 31/10/2024 14:58, shunxi zhang wrote:
> > From: Shunxi Zhang <ot_shunxi.zhang@xxxxxxxxxxxx>
> >
>
> Missing commit msg.
>
> > Signed-off-by: Shunxi Zhang <ot_shunxi.zhang@xxxxxxxxxxxx>
> > ---
> > drivers/rtc/Kconfig | 10 +
> > drivers/rtc/Makefile | 1 +
> > drivers/rtc/rtc-mt6685.c | 1456
> > ++++++++++++++++++++++++++
> > include/linux/mfd/mt6685-audclk.h | 11 +
> > include/linux/mfd/mt6685/core.h | 22 +
> > include/linux/mfd/mt6685/registers.h | 921 ++++++++++++++++
> > include/linux/mfd/mt6685/rtc.h | 318 ++++++
>
> Two separate drivers should not be in one commit.
>
> > 7 files changed, 2739 insertions(+)
> > create mode 100644 drivers/rtc/rtc-mt6685.c
> > create mode 100644 include/linux/mfd/mt6685-audclk.h
> > create mode 100644 include/linux/mfd/mt6685/core.h
> > create mode 100644 include/linux/mfd/mt6685/registers.h
> > create mode 100644 include/linux/mfd/mt6685/rtc.h
> >
> > diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> > index 66eb1122248b..7af04dfac978 100644
> > --- a/drivers/rtc/Kconfig
> > +++ b/drivers/rtc/Kconfig
> > @@ -1898,6 +1898,16 @@ config RTC_DRV_MT6397
> >
> > If you want to use MediaTek(R) RTC interface, select Y or M
> > here.
> >
> > +config RTC_DRV_MT6685
> > + tristate "Mediatek Real Time Clock driver"
> > + depends on MFD_MT6685 || (COMPILE_TEST && IRQ_DOMAIN)
> > + help
> > + This selects the Mediatek(R) RTC driver. RTC is part of
> > Mediatek
> > + MT6685. You should enable MT6685 MFD before select
> > + Mediatek(R) RTC driver.
> > +
> > + If you want to use Mediatek(R) RTC interface, select Y or M
> > here.
> > +
> > config RTC_DRV_MT7622
> > tristate "MediaTek SoC based RTC"
> > depends on ARCH_MEDIATEK || COMPILE_TEST
> > diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
> > index f62340ecc534..ec982192526d 100644
> > --- a/drivers/rtc/Makefile
> > +++ b/drivers/rtc/Makefile
> > @@ -110,6 +110,7 @@ obj-$(CONFIG_RTC_DRV_SSD202D) += rtc-
> > ssd202d.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_MT6685) += rtc-mt6685.o
> > obj-$(CONFIG_RTC_DRV_MT7622) += rtc-mt7622.o
> > obj-$(CONFIG_RTC_DRV_MV) += rtc-mv.o
> > obj-$(CONFIG_RTC_DRV_MXC) += rtc-mxc.o
> > diff --git a/drivers/rtc/rtc-mt6685.c b/drivers/rtc/rtc-mt6685.c
>
> Why this cannot be part of existing MT drivers?
>
> > new file mode 100644
> > index 000000000000..a3aa747a788a
> > --- /dev/null
> > +++ b/drivers/rtc/rtc-mt6685.c
> > @@ -0,0 +1,1456 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2020 MediaTek Inc.
> > + * Author: Amber Lin <Mw.lin@xxxxxxxxxxxx>
> > + */
> > +
> > +#include <linux/err.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/module.h>
> > +#include <linux/mutex.h>
> > +#include <linux/of_device.h>
> > +#include <linux/of_irq.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/regmap.h>
> > +#include <linux/rtc.h>
> > +#include <linux/mfd/mt6685/rtc.h>
> > +#include <linux/mfd/mt6685/core.h>
> > +#include <linux/mfd/mt6685/registers.h>
> > +#include <linux/nvmem-provider.h>
> > +#include <linux/sched/clock.h>
> > +#include <linux/spmi.h>
> > +
> > +#if IS_ENABLED(CONFIG_MTK_AEE_FEATURE)
>
> There is no such stuff. Don't send us downstream code.
>
> > +#include <mt-plat/aee.h>
> > +#endif
> > +
> > +#ifdef SUPPORT_PWR_OFF_ALARM
>
> ???
>
> > +#include <linux/notifier.h>
> > +#include <linux/suspend.h>
> > +#include <linux/completion.h>
> > +#include <linux/workqueue.h>
> > +#include <linux/jiffies.h>
> > +#include <linux/cpumask.h>
> > +#include <linux/reboot.h>
>
> Drop all unrelated, unused headers.
>
> > +#endif
> > +
> > +/*debug information*/
> > +static int rtc_show_time;
> > +static int rtc_show_alarm;
> > +
> > +module_param(rtc_show_time, int, 0644);
> > +module_param(rtc_show_alarm, int, 0644);
>
> Drop all four. What's the purpose?
>
> > +
> > +static int mtk_rtc_write_trigger(struct mt6685_rtc *rtc);
> > +
> > +static int counter;
>
> Drop.
>
>
> > +
> > +static void power_on_mclk(struct mt6685_rtc *rtc)
> > +{
> > + mutex_lock(&rtc->clk_lock);
> > + /*Power on RTC MCLK and 32k clk before write RTC register*/
>
> This code has quite poor quality. I think you just send whatever you
> had
> downstream. Code has multiple trivial issues which should be easily
> pointed out by your internal review. Also multiple issues which we
> already fixed in upstream and you are supposed to take *upstream*
> driver, not downstream, and customize it. Please clean it up and
> consult
> internal review before sending such stuff upstream.
>
>
> > + regmap_write(rtc->regmap, RG_RTC_32K_CK_PDN_CLR,
> > RG_RTC_32K_CK_PDN_MASK);
> > + regmap_write(rtc->regmap, RG_RTC_MCLK_PDN_CLR,
> > RG_RTC_MCLK_PDN_MASK);
> > + counter++;
> > + mdelay(1);
> > + mutex_unlock(&rtc->clk_lock);
> > +}
> > +
> > +static void power_down_mclk(struct mt6685_rtc *rtc)
> > +{
> > + mutex_lock(&rtc->clk_lock);
> > + counter--;
> > + if (counter < 0) {
> > + //dump_stack();
> > + pr_info("mclk_counter[%d]\n", counter);
>
> Oh man... So many wrong things. This applies to entire code:
> 1. Drop dead code. All dead code.
>
> 2. Do not use pr_xxx but dev_xxx
>
> 3. Drop all such useless printks because your driver is supposed to
> be
> silent.
>
> 4. Implement proper clock handling instead of reimplementing it
> yourself
> with some counters.
>
> ...
>
> > +
> > +static const struct reg_field
> > mt6685_cali_reg_fields[CALI_FILED_MAX] = {
> > + [EOSC_CALI_TD] = REG_FIELD(EOSC_CALI_TD_MT6685, 0,
> > 2),
> > +};
> > +
> > +static int rtc_eosc_cali_td;
> > +module_param(rtc_eosc_cali_td, int, 0644);
>
> Why do you put module params in multiple places? This is supposed to
> be
> in one, top level place, so we see all such interfaces.
>
> Anyway, drop.
>
>
> > +
> > +static void mtk_rtc_enable_k_eosc(struct device *dev)
> > +{
> > + struct mt6685_rtc *rtc = dev_get_drvdata(dev);
> > + u32 td;
> > +
> > + power_on_mclk(rtc);
>
> This code is really suspicious.
>
> > +
> > + if (!rtc->cali_is_supported) {
> > + power_down_mclk(rtc);
> > + return;
> > + }
> > +
> > + if (rtc_eosc_cali_td) {
> > + dev_notice(dev, "%s: rtc_eosc_cali_td = %d\n",
> > + __func__, rtc_eosc_cali_td);
>
> No, drop.
>
> > + switch (rtc_eosc_cali_td) {
> > + case 1:
> > + td = EOSC_CALI_TD_01_SEC;
> > + break;
> > + case 2:
> > + td = EOSC_CALI_TD_02_SEC;
> > + break;
> > + case 4:
> > + td = EOSC_CALI_TD_04_SEC;
> > + break;
> > + case 16:
> > + td = EOSC_CALI_TD_16_SEC;
> > + break;
> > + default:
> > + td = EOSC_CALI_TD_08_SEC;
> > + break;
> > + }
> > +
> > + rtc_field_write(rtc, &rtc->data-
> > >cali_reg_fields[EOSC_CALI_TD], td);
> > + }
> > + power_down_mclk(rtc);
> > +}
> > +
> > +#ifdef SUPPORT_PWR_OFF_ALARM
>
> Is this some sort of joke? There is no such thing.
>
>
> > +
> > +static u32 bootmode = NORMAL_BOOT;
> > +static struct wakeup_source *mt6685_rtc_suspend_lock;
> > +#if IS_ENABLED(CONFIG_PM)
> > +static bool rtc_pm_notifier_registered;
> > +static unsigned long rtc_pm_status;
> > +#endif
> > +static bool kpoc_alarm;
> > +
> > +#if IS_ENABLED(CONFIG_PM)
> > +
> > +#define PM_DUMMY 0xFFFF
>
> NAK to all above.
>
> > +
> > +static int rtc_pm_event(struct notifier_block *notifier, unsigned
> > long pm_event,
> > + void *unused)
> > +{
> > + struct mt6685_rtc *rtc = container_of(notifier,
> > + struct mt6685_rtc, pm_nb);
> > +
> > + switch (pm_event) {
> > + case PM_SUSPEND_PREPARE:
> > + rtc_pm_status = PM_SUSPEND_PREPARE;
> > + return NOTIFY_DONE;
> > + case PM_POST_SUSPEND:
> > + rtc_pm_status = PM_POST_SUSPEND;
> > + break;
> > + default:
> > + rtc_pm_status = PM_DUMMY;
> > + break;
> > + }
> > +
> > + if (kpoc_alarm) {
> > + dev_notice(rtc->rtc_dev->dev.parent,
> > + "%s trigger reboot\n", __func__);
> > + complete(&rtc->comp);
> > + kpoc_alarm = false;
> > + }
> > + return NOTIFY_DONE;
> > +}
> > +#endif /* CONFIG_PM */
> > +
> > +static void rtc_mark_kpoc(struct mt6685_rtc *rtc)
> > +{
> > + power_on_mclk(rtc);
> > + mutex_lock(&rtc->lock);
> > + rtc_field_write(rtc, &rtc->data-
> > >spare_reg_fields[SPARE_KPOC], 1);
> > + mutex_unlock(&rtc->lock);
> > + power_down_mclk(rtc);
> > +}
> > +
> > +static void mtk_rtc_work_queue(struct work_struct *work)
> > +{
> > + struct mt6685_rtc *rtc = container_of(work, struct
> > mt6685_rtc, work);
> > + unsigned long ret;
> > + unsigned int msecs;
> > +
> > + ret = wait_for_completion_timeout(&rtc->comp,
> > msecs_to_jiffies(30000));
> > + if (!ret) {
> > + dev_notice(rtc->rtc_dev->dev.parent, "%s timeout\n",
> > __func__);
> > + WARN_ON(1);
> > + } else {
> > + msecs = jiffies_to_msecs(ret);
> > + dev_notice(rtc->rtc_dev->dev.parent,
> > + "%s timeleft= %d\n", __func__, msecs);
> > + rtc_mark_kpoc(rtc);
> > + kernel_restart("kpoc");
> > + }
> > +}
> > +
> > +static void mtk_rtc_reboot(struct mt6685_rtc *rtc)
> > +{
> > + __pm_stay_awake(mt6685_rtc_suspend_lock);
> > +
> > + init_completion(&rtc->comp);
> > + schedule_work_on(cpumask_first(cpu_online_mask), &rtc->work);
> > +
> > +#if IS_ENABLED(CONFIG_PM)
> > + if (!rtc_pm_notifier_registered)
> > + goto reboot;
> > +
> > + if (rtc_pm_status != PM_SUSPEND_PREPARE)
> > + goto reboot;
> > +#endif
> > +
> > + kpoc_alarm = true;
> > +
> > + dev_notice(rtc->rtc_dev->dev.parent, "%s:wait\n", __func__);
> > + return;
> > +
> > +#if IS_ENABLED(CONFIG_PM)
> > +reboot:
> > + dev_notice(rtc->rtc_dev->dev.parent, "%s:trigger\n",
> > __func__);
> > + complete(&rtc->comp);
> > +#endif
> > +}
> > +
> > +static void mtk_rtc_update_pwron_alarm_flag(struct mt6685_rtc
> > *rtc)
> > +{
> > + int ret;
> > +
> > + power_on_mclk(rtc);
> > +
> > + ret = rtc_update_bits(rtc,
> > + rtc->addr_base + RTC_PDN1,
> > + RTC_PDN1_PWRON_TIME, 0);
> > + if (ret < 0)
> > + goto exit;
> > +
> > + ret = rtc_update_bits(rtc,
> > + rtc->addr_base + RTC_PDN2,
> > + RTC_PDN2_PWRON_ALARM,
> > RTC_PDN2_PWRON_ALARM);
> > + if (ret < 0)
> > + goto exit;
> > +
> > + mtk_rtc_write_trigger(rtc);
> > + power_down_mclk(rtc);
> > + dev_notice(rtc->rtc_dev->dev.parent, "%s info\n", __func__);
>
> NAK
>
> > + return;
> > +
> > +exit:
> > + power_down_mclk(rtc);
> > + dev_err(rtc->rtc_dev->dev.parent, "%s error\n", __func__);
>
> NAK, what error? Be descriptive.
>
>
> > +}
> > +
> > +static int mtk_rtc_restore_alarm(struct mt6685_rtc *rtc, struct
> > rtc_time *tm)
> > +{
> > + int ret;
> > + u16 data[RTC_OFFSET_COUNT] = { 0 };
> > +
> > + power_on_mclk(rtc);
> > +
> > + ret = rtc_bulk_read(rtc, rtc->addr_base + RTC_AL_SEC,
> > + data, RTC_OFFSET_COUNT * 2);
> > + if (ret < 0)
> > + goto exit;
> > +
> > + data[RTC_OFFSET_SEC] = ((data[RTC_OFFSET_SEC] &
> > ~(RTC_AL_SEC_MASK)) |
> > + (tm->tm_sec & RTC_AL_SEC_MASK));
> > + data[RTC_OFFSET_MIN] = ((data[RTC_OFFSET_MIN] &
> > ~(RTC_AL_MIN_MASK)) |
> > + (tm->tm_min & RTC_AL_MIN_MASK));
> > + data[RTC_OFFSET_HOUR] = ((data[RTC_OFFSET_HOUR] &
> > ~(RTC_AL_HOU_MASK)) |
> > + (tm->tm_hour & RTC_AL_HOU_MASK));
> > + data[RTC_OFFSET_DOM] = ((data[RTC_OFFSET_DOM] &
> > ~(RTC_AL_DOM_MASK)) |
> > + (tm->tm_mday & RTC_AL_DOM_MASK));
> > + data[RTC_OFFSET_MTH] = ((data[RTC_OFFSET_MTH] &
> > ~(RTC_AL_MTH_MASK)) |
> > + (tm->tm_mon & RTC_AL_MTH_MASK));
> > + data[RTC_OFFSET_YEAR] = ((data[RTC_OFFSET_YEAR] &
> > ~(RTC_AL_YEA_MASK)) |
> > + (tm->tm_year & RTC_AL_YEA_MASK));
> > +
> > + dev_notice(rtc->rtc_dev->dev.parent,
> > + "restore al time = %04d/%02d/%02d
> > %02d:%02d:%02d\n",
> > + tm->tm_year + RTC_MIN_YEAR, tm->tm_mon, tm-
> > >tm_mday,
> > + tm->tm_hour, tm->tm_min, tm->tm_sec);
> > +
> > + ret = rtc_bulk_write(rtc, rtc->addr_base + RTC_AL_SEC,
> > + data, RTC_OFFSET_COUNT * 2);
> > + if (ret < 0)
> > + goto exit;
> > +
> > + ret = rtc_write(rtc, rtc->addr_base + RTC_AL_MASK,
> > + RTC_AL_MASK_DOW);
> > + if (ret < 0)
> > + goto exit;
> > +
> > + ret = rtc_update_bits(rtc,
> > + rtc->addr_base + RTC_IRQ_EN,
> > + RTC_IRQ_EN_AL, RTC_IRQ_EN_AL);
> > + if (ret < 0)
> > + goto exit;
> > +
> > + mtk_rtc_write_trigger(rtc);
> > + power_down_mclk(rtc);
> > + return ret;
> > +
> > +exit:
> > + power_down_mclk(rtc);
> > + dev_err(rtc->rtc_dev->dev.parent, "%s error\n", __func__);
>
> Again, be descriptive.
>
> > + return ret;
> > +}
> > +
> > +static bool mtk_rtc_is_pwron_alarm(struct mt6685_rtc *rtc,
> > + struct rtc_time *nowtm, struct
> > rtc_time *tm)
> > +{
> > + u32 pdn1 = 0, spar1 = 0, pdn2 = 0, spar0 = 0;
> > + int ret, sec = 0;
> > + u16 data[RTC_OFFSET_COUNT] = { 0 };
> > +
> > + ret = rtc_read(rtc, rtc->addr_base + RTC_PDN1, &pdn1);
> > + if (ret < 0)
> > + goto exit;
> > +
> > + dev_notice(rtc->rtc_dev->dev.parent, "pdn1 = 0x%x\n", pdn1);
>
> NAK
>
>
> ...
>
>
> > +
> > + dev_info(rtc->rtc_dev->dev.parent,
> > + "[HWID 0x%x, MCLK 0x%x, prot
> > key 0x%x] %s write %d, latest %d\n",
> > + hwid, mclk, prot_key,
> > rtc_time_reg_name[i],
> > + data[i], latest[i]);
> > + }
>
> NAK
>
> > + }
> > +
> > + if (write_fail > 0)
> > + mdelay(2);
> > + else
> > + break;
> > + }
> > +
> > + if (write_fail > 0)
> > + return false;
> > +
> > + return true;
> > +}
> > +
> > +static int mtk_rtc_set_time(struct device *dev, struct rtc_time
> > *tm)
> > +{
> > + struct mt6685_rtc *rtc = dev_get_drvdata(dev);
> > + int ret, result = 0;
> > + u16 data[RTC_OFFSET_COUNT];
> > +
> > + power_on_mclk(rtc);
> > +
> > + dev_notice(rtc->rtc_dev->dev.parent,
> > + "set tc time = %04d/%02d/%02d %02d:%02d:%02d\n",
> > + tm->tm_year + RTC_BASE_YEAR, tm->tm_mon + 1, tm-
> > >tm_mday,
> > + tm->tm_hour, tm->tm_min, tm->tm_sec);
> > +
> > + tm->tm_year -= RTC_MIN_YEAR_OFFSET;
> > + tm->tm_mon++;
> > +
> > + data[RTC_OFFSET_SEC] = tm->tm_sec;
> > + data[RTC_OFFSET_MIN] = tm->tm_min;
> > + data[RTC_OFFSET_HOUR] = tm->tm_hour;
> > + data[RTC_OFFSET_DOM] = tm->tm_mday;
> > + data[RTC_OFFSET_MTH] = tm->tm_mon;
> > + data[RTC_OFFSET_YEAR] = tm->tm_year;
> > +
> > + mutex_lock(&rtc->lock);
> > + ret = rtc_bulk_write(rtc, rtc->addr_base + RTC_TC_SEC,
> > + data, RTC_OFFSET_COUNT * 2);
> > + if (ret < 0)
> > + goto exit;
> > +
> > + /* Time register write to hardware after call trigger
> > function */
> > + ret = mtk_rtc_write_trigger(rtc);
> > + if (ret < 0)
> > + goto exit;
> > +
> > + result = mtk_rtc_check_set_time(rtc, tm, 2, RTC_TC_SEC);
> > +
> > + if (!result) {
> > + dev_info(rtc->rtc_dev->dev.parent, "check rtc set
> > time\n");
> > +#if IS_ENABLED(CONFIG_MTK_AEE_FEATURE)
> > + aee_kernel_warning("mt6685-rtc", "mt6685-rtc: set
> > tick time failed\n");
> > +#endif
>
> NAK
>
> > + }
> > +
> > +exit:
> > + mutex_unlock(&rtc->lock);
> > + power_down_mclk(rtc);
>
> I did not investigate locking but it feels like you lock everything
> everywhere.
>
> Considering how poor code is this, I suspect that locking is totally
> bogus. Provide extensive description of locking in comment section in
> the top.
>
>
> > + return ret;
> > +}
> > +
> > +static int mtk_rtc_read_alarm(struct device *dev, struct
> > rtc_wkalrm *alm)
> > +{
> > + struct rtc_time *tm = &alm->time;
> > + struct mt6685_rtc *rtc = dev_get_drvdata(dev);
> > + u32 irqen = 0, pdn2 = 0;
> > + int ret;
> > + u16 data[RTC_OFFSET_COUNT] = { 0 };
> > +
> > + mutex_lock(&rtc->lock);
> > + ret = rtc_read(rtc, rtc->addr_base + RTC_IRQ_EN, &irqen);
> > + if (ret < 0)
> > + goto err_exit;
> > + ret = rtc_read(rtc, rtc->addr_base + RTC_PDN2, &pdn2);
> > + if (ret < 0)
> > + goto err_exit;
> > +
> > + ret = rtc_bulk_read(rtc, rtc->addr_base + RTC_AL_SEC,
> > + data, RTC_OFFSET_COUNT * 2);
> > + if (ret < 0)
> > + goto err_exit;
> > +
> > + alm->enabled = !!(irqen & RTC_IRQ_EN_AL);
> > + alm->pending = !!(pdn2 & RTC_PDN2_PWRON_ALARM);
> > + mutex_unlock(&rtc->lock);
> > +
> > + tm->tm_sec = data[RTC_OFFSET_SEC] & RTC_AL_SEC_MASK;
> > + tm->tm_min = data[RTC_OFFSET_MIN] & RTC_AL_MIN_MASK;
> > + tm->tm_hour = data[RTC_OFFSET_HOUR] & RTC_AL_HOU_MASK;
> > + tm->tm_mday = data[RTC_OFFSET_DOM] & RTC_AL_DOM_MASK;
> > + tm->tm_mon = data[RTC_OFFSET_MTH] & RTC_AL_MTH_MASK;
> > + tm->tm_year = data[RTC_OFFSET_YEAR] & RTC_AL_YEA_MASK;
> > +
> > + tm->tm_year += RTC_MIN_YEAR_OFFSET;
> > + tm->tm_mon--;
> > +
> > + dev_notice(rtc->rtc_dev->dev.parent,
> > + "read al time = %04d/%02d/%02d %02d:%02d:%02d
> > (%d)\n",
> > + tm->tm_year + RTC_BASE_YEAR, tm->tm_mon + 1, tm-
> > >tm_mday,
> > + tm->tm_hour, tm->tm_min, tm->tm_sec, alm-
> > >enabled);
> > +
> > + return 0;
> > +err_exit:
> > + mutex_unlock(&rtc->lock);
> > + return ret;
> > +}
> > +
> > +static int mtk_rtc_set_alarm(struct device *dev, struct rtc_wkalrm
> > *alm)
> > +{
> > + struct rtc_time *tm = &alm->time;
> > + struct mt6685_rtc *rtc = dev_get_drvdata(dev);
> > + int ret, result = 0;
> > + u16 data[RTC_OFFSET_COUNT];
> > + ktime_t target;
> > +
> > + power_on_mclk(rtc);
> > +
> > + if (alm->enabled == 1) {
> > + /* Add one more second to postpone wake time. */
> > + target = rtc_tm_to_ktime(*tm);
> > + target = ktime_add_ns(target, NSEC_PER_SEC);
> > + *tm = rtc_ktime_to_tm(target);
> > + }
> > +
> > + tm->tm_year -= RTC_MIN_YEAR_OFFSET;
> > + tm->tm_mon++;
> > +
> > + dev_notice(rtc->rtc_dev->dev.parent,
> > + "set al time = %04d/%02d/%02d %02d:%02d:%02d
> > (%d)\n",
> > + tm->tm_year + RTC_MIN_YEAR, tm->tm_mon, tm-
> > >tm_mday,
> > + tm->tm_hour, tm->tm_min, tm->tm_sec, alm-
> > >enabled);
> > +
> > + mutex_lock(&rtc->lock);
> > +
> > + switch (alm->enabled) {
> > + case 3:
> > + /* enable power-on alarm with logo */
> > + mtk_rtc_save_pwron_time(rtc, true, tm);
> > + break;
> > + case 4:
> > + /* disable power-on alarm */
> > + mtk_rtc_save_pwron_time(rtc, false, tm);
> > + break;
> > + default:
> > + break;
> > + }
> > +
> > + ret = rtc_update_bits(rtc,
> > + rtc->addr_base + RTC_PDN2,
> > + RTC_PDN2_PWRON_ALARM, 0);
> > + if (ret < 0)
> > + goto exit;
> > + mtk_rtc_write_trigger(rtc);
> > +
> > + ret = rtc_bulk_read(rtc, rtc->addr_base + RTC_AL_SEC,
> > + data, RTC_OFFSET_COUNT * 2);
> > + if (ret < 0)
> > + goto exit;
> > +
> > + data[RTC_OFFSET_SEC] = ((data[RTC_OFFSET_SEC] &
> > ~(RTC_AL_SEC_MASK)) |
> > + (tm->tm_sec & RTC_AL_SEC_MASK));
> > + data[RTC_OFFSET_MIN] = ((data[RTC_OFFSET_MIN] &
> > ~(RTC_AL_MIN_MASK)) |
> > + (tm->tm_min & RTC_AL_MIN_MASK));
> > + data[RTC_OFFSET_HOUR] = ((data[RTC_OFFSET_HOUR] &
> > ~(RTC_AL_HOU_MASK)) |
> > + (tm->tm_hour & RTC_AL_HOU_MASK));
> > + data[RTC_OFFSET_DOM] = ((data[RTC_OFFSET_DOM] &
> > ~(RTC_AL_DOM_MASK)) |
> > + (tm->tm_mday & RTC_AL_DOM_MASK));
> > + data[RTC_OFFSET_MTH] = ((data[RTC_OFFSET_MTH] &
> > ~(RTC_AL_MTH_MASK)) |
> > + (tm->tm_mon & RTC_AL_MTH_MASK));
> > + data[RTC_OFFSET_YEAR] = ((data[RTC_OFFSET_YEAR] &
> > ~(RTC_AL_YEA_MASK)) |
> > + (tm->tm_year & RTC_AL_YEA_MASK));
> > +
> > + if (alm->enabled) {
> > + ret = rtc_bulk_write(rtc,
> > + rtc->addr_base + RTC_AL_SEC,
> > + data, RTC_OFFSET_COUNT * 2);
> > + if (ret < 0)
> > + goto exit;
> > + ret = rtc_write(rtc, rtc->addr_base + RTC_AL_MASK,
> > + RTC_AL_MASK_DOW);
> > + if (ret < 0)
> > + goto exit;
> > +
> > + ret = rtc_update_bits(rtc,
> > + rtc->addr_base + RTC_IRQ_EN,
> > + RTC_IRQ_EN_AL, RTC_IRQ_EN_AL);
> > + if (ret < 0)
> > + goto exit;
> > + } else {
> > + ret = rtc_update_bits(rtc,
> > + rtc->addr_base +
> > RTC_IRQ_EN,
> > + RTC_IRQ_EN_AL, 0);
> > + if (ret < 0)
> > + goto exit;
> > + }
> > +
> > + /* All alarm time register write to hardware after calling
> > + * mtk_rtc_write_trigger. This can avoid race condition if
> > alarm
> > + * occur happen during writing alarm time register.
> > + */
> > + ret = mtk_rtc_write_trigger(rtc);
> > + if (ret < 0)
> > + goto exit;
> > +
> > + result = mtk_rtc_check_set_time(rtc, tm, 2, RTC_AL_SEC);
> > +
> > + if (!result) {
> > + dev_info(rtc->rtc_dev->dev.parent, "check rtc set
> > alarm\n");
> > +#if IS_ENABLED(CONFIG_MTK_AEE_FEATURE)
> > + aee_kernel_warning("mt6685-rtc", "mt6685-rtc: set
> > alarm time failed\n");
> > +#endif
> > + }
> > +exit:
> > + mutex_unlock(&rtc->lock);
> > + power_down_mclk(rtc);
> > + return ret;
> > +}
> > +
> > +static int rtc_alarm_set_power_on(struct device *dev, struct
> > rtc_wkalrm *alm)
> > +{
> > + int err = 0;
> > + struct rtc_time tm;
> > + time64_t now, scheduled;
> > +
> > + err = rtc_valid_tm(&alm->time);
> > + if (err != 0)
> > + return err;
> > + scheduled = rtc_tm_to_time64(&alm->time);
> > +
> > + err = mtk_rtc_read_time(dev, &tm);
> > + if (err != 0)
> > + return err;
> > + now = rtc_tm_to_time64(&tm);
> > +
> > + if (scheduled <= now)
> > + alm->enabled = 4;
> > + else
> > + alm->enabled = 3;
> > +
> > + mtk_rtc_set_alarm(dev, alm);
> > +
> > + return err;
> > +}
> > +
> > +static int mtk_rtc_ioctl(struct device *dev, unsigned int cmd,
> > unsigned long arg)
> > +{
> > + void __user *uarg = (void __user *)arg;
> > + int err = 0;
> > + struct rtc_wkalrm alm;
> > +
> > + switch (cmd) {
> > + case RTC_POFF_ALM_SET:
> > + if (copy_from_user(&alm.time, uarg,
> > sizeof(alm.time)))
> > + return -EFAULT;
> > + err = rtc_alarm_set_power_on(dev, &alm);
> > + break;
> > + default:
> > + err = -EINVAL;
> > + break;
> > + }
> > +
> > + return err;
> > +}
> > +
> > +static const struct rtc_class_ops mtk_rtc_ops = {
> > + .ioctl = mtk_rtc_ioctl,
> > + .read_time = mtk_rtc_read_time,
> > + .set_time = mtk_rtc_set_time,
> > + .read_alarm = mtk_rtc_read_alarm,
> > + .set_alarm = mtk_rtc_set_alarm,
> > +};
> > +
> > +static int rtc_nvram_read(void *priv, unsigned int offset, void
> > *val,
> > + size_t bytes)
> > +{
> > + struct mt6685_rtc *rtc = dev_get_drvdata(priv);
> > + unsigned int ival;
> > + int ret;
> > + u8 *buf = val;
> > +
> > + mutex_lock(&rtc->lock);
> > +
> > + for (; bytes; bytes--) {
> > + ret = rtc_field_read(rtc,
> > + &rtc->data-
> > >spare_reg_fields[offset++],
> > + &ival);
> > +
> > + if (ret)
> > + goto out;
> > +
> > + *buf++ = (u8)ival;
> > + }
> > +
> > +out:
> > + mutex_unlock(&rtc->lock);
> > + return ret;
> > +}
> > +
> > +static int rtc_nvram_write(void *priv, unsigned int offset, void
> > *val,
> > + size_t bytes)
> > +{
> > + struct mt6685_rtc *rtc = dev_get_drvdata(priv);
> > + unsigned int ival;
> > + int ret;
> > + u8 *buf = val;
> > +
> > + power_on_mclk(rtc);
> > +
> > + mutex_lock(&rtc->lock);
> > +
> > + for (; bytes; bytes--) {
> > + ival = *buf++;
> > + ret = rtc_field_write(rtc,
> > + &rtc->data-
> > >spare_reg_fields[offset++],
> > + ival);
> > +
> > + if (ret)
> > + goto out;
> > + }
> > +
> > +out:
> > + mutex_unlock(&rtc->lock);
> > + power_down_mclk(rtc);
> > + return ret;
> > +}
> > +
> > +static int mtk_rtc_set_spare(struct device *dev)
> > +{
> > + struct mt6685_rtc *rtc = dev_get_drvdata(dev);
> > + int ret;
> > + struct nvmem_config nvmem_cfg = {
> > + .name = "mtk_rtc_nvmem",
> > + .word_size = SPARE_REG_WIDTH,
> > + .stride = 1,
> > + .size = SPARE_RG_MAX * SPARE_REG_WIDTH,
> > + .reg_read = rtc_nvram_read,
> > + .reg_write = rtc_nvram_write,
> > + .priv = dev,
> > + };
> > +
> > + ret = devm_rtc_nvmem_register(rtc->rtc_dev, &nvmem_cfg);
> > + if (ret)
> > + dev_err(rtc->rtc_dev->dev.parent, "nvmem register
> > failed\n");
> > +
> > + return ret;
> > +}
> > +
> > +static int mtk_rtc_probe(struct platform_device *pdev)
> > +{
> > + struct mt6685_rtc *rtc;
> > + struct device_node *np = pdev->dev.of_node;
> > + int ret;
> > +#ifdef SUPPORT_PWR_OFF_ALARM
> > + struct device_node *of_chosen = NULL;
> > + struct tag_bootmode *tag = NULL;
> > +#endif
> > + if (!of_device_is_available(np)) {
> > + dev_err(&pdev->dev, "rtc disabled\n");
>
> NAK, this code is a disaster.
>
> > + return -1;
> > + }
> > +
> > + rtc = devm_kzalloc(&pdev->dev, sizeof(struct mt6685_rtc),
> > GFP_KERNEL);
> > + if (!rtc) {
> > + //dev_err(&pdev->dev, "devm_kzalloc failed\n");
>
> NAK
>
>
> > + return -ENOMEM;
> > + }
> > +
> > + rtc->data = of_device_get_match_data(&pdev->dev);
> > + if (!rtc->data) {
> > + dev_err(&pdev->dev, "of_device_get_match_data
> > failed\n");
>
> NAK
>
> > + return -ENODEV;
> > + }
> > +
> > + if (of_property_read_u32(pdev->dev.of_node, "base", &rtc-
> > >addr_base))
> > + rtc->addr_base = RTC_DSN_ID;
> > +
> > + pr_notice("%s: rtc->addr_base =0x%x\n", __func__, rtc-
> > >addr_base);
>
> NAK
>
> I finished here. The code is terrible and not suitable for mainline.
> It's probably overcomplicated for something like RTC, but that's not
> yet
> the main problem and I did not get there yet.
>
> Drop all this code. Sorry, it's just not started correctly. Instead
> take
> RECENT mainline driver and customize it using upstream, not
> downstream,
> patterns and coding style.
>
> Best regards,
> Krzysztof
>
