Re: [PATCH v2 4/5] rtc: mt6397: Add BBPU alarm status reset and shutdown handling

["Shunxi Zhang (章顺喜)" <ot_shunxi.zhang@mediatek.com>]

On Thu, 2025-11-20 at 15:42 +0100, AngeloGioacchino Del Regno wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> Il 20/11/25 13:18, ot_shunxi.zhang@mediatek.com ha scritto:
> > From: Shunxi Zhang <ot_shunxi.zhang@mediatek.com>
> > 
> > Function "mtk_rtc_reset_bbpu_alarm_status" is added to address the
> > issue that the RTC BBPU alarm state remains after the RTC alarm
> > has occurred.
> > 
> > Additionally, function "mtk_rtc_shutdown" is added to address the
> > issue of the platform being powered on again after shutdown because
> > the RTC_BBPU alarm state was not cleared.
> > 
> > Signed-off-by: Shunxi Zhang <ot_shunxi.zhang@mediatek.com>
> > ---
> >   drivers/rtc/rtc-mt6397.c | 30 ++++++++++++++++++++++++++++++
> >   1 file changed, 30 insertions(+)
> > 
> > diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
> > index b8f44a00de5d..8bf7e0822ef0 100644
> > --- a/drivers/rtc/rtc-mt6397.c
> > +++ b/drivers/rtc/rtc-mt6397.c
> > @@ -37,6 +37,21 @@ static int mtk_rtc_write_trigger(struct
> > mt6397_rtc *rtc)
> >       return ret;
> >   }
> > 
> > +static void mtk_rtc_reset_bbpu_alarm_status(struct mt6397_rtc
> > *rtc)
> > +{
> > +     u32 bbpu = RTC_BBPU_KEY | RTC_BBPU_PWREN | RTC_BBPU_RESET_AL;
> > +     int ret;
> > +
> > +     ret = regmap_write(rtc->regmap, rtc->addr_base + RTC_BBPU,
> > bbpu);
> > +     if (ret < 0) {
> > +             dev_err(rtc->rtc_dev->dev.parent, "%s: write rtc bbpu
> > error\n",
> > +                     __func__);
> > +             return;
> > +     }
> > +
> > +     mtk_rtc_write_trigger(rtc);
> > +}
> > +
> >   static irqreturn_t mtk_rtc_irq_handler_thread(int irq, void
> > *data)
> >   {
> >       struct mt6397_rtc *rtc = data;
> > @@ -51,6 +66,9 @@ static irqreturn_t mtk_rtc_irq_handler_thread(int
> > irq, void *data)
> >               if (regmap_write(rtc->regmap, rtc->addr_base +
> > RTC_IRQ_EN,
> >                                irqen) == 0)
> >                       mtk_rtc_write_trigger(rtc);
> > +
> > +             if (rtc->alarm_sta_supported)
> > +                     mtk_rtc_reset_bbpu_alarm_status(rtc);
> >               mutex_unlock(&rtc->lock);
> > 
> >               return IRQ_HANDLED;
> > @@ -249,6 +267,7 @@ static int mtk_rtc_probe(struct platform_device
> > *pdev)
> >       struct resource *res;
> >       struct mt6397_chip *mt6397_chip = dev_get_drvdata(pdev-
> > >dev.parent);
> >       struct mt6397_rtc *rtc;
> > +     struct device_node *np = pdev->dev.of_node;
> >       int ret;
> > 
> >       rtc = devm_kzalloc(&pdev->dev, sizeof(struct mt6397_rtc),
> > GFP_KERNEL);
> > @@ -275,6 +294,8 @@ static int mtk_rtc_probe(struct platform_device
> > *pdev)
> >       if (IS_ERR(rtc->rtc_dev))
> >               return PTR_ERR(rtc->rtc_dev);
> > 
> > +     rtc->alarm_sta_supported = of_property_read_bool(np,
> > "mediatek,alarm-sta-supported");
> 
> You don't need a DT property - the PMIC dictates support for that,
> not the board.
> 
> This means that you also don't need the alarm_sta_supported variable,
> and you
> don't need to check for it.
> 
> Just execute the mtk_rtc_reset_bbpu_alarm_status() function when you
> have to,
> without any check.
> 
> Cheers,
> Angelo

Dear sir,
Thanks for your comment. This check is base on the last comment(
https://patchwork.kernel.org/project/linux-mediatek/patch/20250811081543.4377-2-ot_shunxi.zhang@mediatek.com/#26517538
) reply:
"The MT6397 is an integration of several ICs and does not have a
separate IC specification. I will check the relevant IC datasheets
again. I will remove the useless define in next version".

Only the MT6359 has the "BBPU alarm status" function, so this attribute
needs to be added to the RTC function in MT6359.dtsi for judgment. This
ensures compatibility with other PMICs (MT6358, etc.) in rtc-mt6397.c.

Excuse me, do you have any further suggestions on better compatibility?
I will improve and modify it in the next version.

Best Regards
Shunxi Zhang

> 
> > +
> >       ret = devm_request_threaded_irq(&pdev->dev, rtc->irq, NULL,
> >                                       mtk_rtc_irq_handler_thread,
> >                                       IRQF_ONESHOT |
> > IRQF_TRIGGER_HIGH,
> > @@ -297,6 +318,14 @@ static int mtk_rtc_probe(struct
> > platform_device *pdev)
> >       return devm_rtc_register_device(rtc->rtc_dev);
> >   }
> > 
> > +static void mtk_rtc_shutdown(struct platform_device *pdev)
> > +{
> > +     struct mt6397_rtc *rtc = platform_get_drvdata(pdev);
> > +
> > +     if (rtc->alarm_sta_supported)
> > +             mtk_rtc_reset_bbpu_alarm_status(rtc);
> > +}
> > +
> >   #ifdef CONFIG_PM_SLEEP
> >   static int mt6397_rtc_suspend(struct device *dev)
> >   {
> > @@ -346,6 +375,7 @@ static struct platform_driver mtk_rtc_driver =
> > {
> >               .pm = &mt6397_pm_ops,
> >       },
> >       .probe = mtk_rtc_probe,
> > +     .shutdown = mtk_rtc_shutdown,
> >   };
> > 
> >   module_platform_driver(mtk_rtc_driver);
> 
>