[v2] rtc: Enhance RTC driver with BBPU bit definitions and shutdown handling

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

Posted by ot_shunxi.zhang@mediatek.com 1 week, 4 days ago

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");
+
 	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);
-- 
2.45.2

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

Posted by AngeloGioacchino Del Regno 1 week, 4 days ago

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

> +
>   	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);

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

Posted by Shunxi Zhang (章顺喜) 1 week, 3 days ago

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);
> 
>

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

Posted by Krzysztof Kozlowski 1 week, 4 days ago

On 20/11/2025 13:18, ot_shunxi.zhang@mediatek.com wrote:
>  
>  	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 cannot add undocumented ABI. Read carefully Linux documentation.

Best regards,
Krzysztof

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

Posted by Shunxi Zhang (章顺喜) 1 week, 3 days ago

On Thu, 2025-11-20 at 14:40 +0100, Krzysztof Kozlowski wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On 20/11/2025 13:18, ot_shunxi.zhang@mediatek.com wrote:
> > 
> >       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 cannot add undocumented ABI. Read carefully Linux documentation.
> 
> Best regards,
> Krzysztof

Dear sir,
  Thanks for your comment, I will read & check linux documentation.
It will be revised in the next version.

Best Regards
Shunxi Zhang