linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 0/2] rtc: Enhance RTC driver with BBPU bit definitions and shutdown handling
@ 2025-08-11  8:15 ot_shunxi.zhang
  2025-08-11  8:15 ` [PATCH v1 1/2] mfd: mt6397: Add new bit definitions for RTC_BBPU register ot_shunxi.zhang
  2025-08-11  8:15 ` [PATCH v1 2/2] rtc: mt6397: Add BBPU alarm status reset and shutdown handling ot_shunxi.zhang
  0 siblings, 2 replies; 11+ messages in thread
From: ot_shunxi.zhang @ 2025-08-11  8:15 UTC (permalink / raw)
  To: Eddie Huang, Sean Wang, Alexandre Belloni, Matthias Brugger,
	Lee Jones, Shunxi Zhang
  Cc: linux-arm-kernel, linux-mediatek, linux-rtc, linux-kernel,
	sirius.wang, vince-wl.liu, jh.hsu

From: Shunxi Zhang <ot_shunxi.zhang@mediatek.com>

(1) Add new bits definitions for RTC_BBPU register, 
(2) Add new functions to reset the BBPU alarm status in mt6397 RTC driver
(3) Add mtk_rtc_shutdown function to handle RTC shutdown events.

Shunxi Zhang (2):
  mfd: mt6397: Add new bit definitions for RTC_BBPU register
  rtc: mt6397: Add BBPU alarm status reset and shutdown handling

 drivers/rtc/rtc-mt6397.c       | 37 ++++++++++++++++++++++++++++++++++
 include/linux/mfd/mt6397/rtc.h |  3 +++
 2 files changed, 40 insertions(+)

-- 
2.45.2


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v1 1/2] mfd: mt6397: Add new bit definitions for RTC_BBPU register
  2025-08-11  8:15 [PATCH v1 0/2] rtc: Enhance RTC driver with BBPU bit definitions and shutdown handling ot_shunxi.zhang
@ 2025-08-11  8:15 ` ot_shunxi.zhang
  2025-08-11 11:02   ` Krzysztof Kozlowski
  2025-08-11  8:15 ` [PATCH v1 2/2] rtc: mt6397: Add BBPU alarm status reset and shutdown handling ot_shunxi.zhang
  1 sibling, 1 reply; 11+ messages in thread
From: ot_shunxi.zhang @ 2025-08-11  8:15 UTC (permalink / raw)
  To: Eddie Huang, Sean Wang, Alexandre Belloni, Matthias Brugger,
	Lee Jones, Shunxi Zhang
  Cc: linux-arm-kernel, linux-mediatek, linux-rtc, linux-kernel,
	sirius.wang, vince-wl.liu, jh.hsu

From: Shunxi Zhang <ot_shunxi.zhang@mediatek.com>

This patch adds new bit definitions for the RTC_BBPU register in the
mt6397 RTC header file. The following bit definitions are introduced:

Signed-off-by: Shunxi Zhang <ot_shunxi.zhang@mediatek.com>
---
 include/linux/mfd/mt6397/rtc.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/linux/mfd/mt6397/rtc.h b/include/linux/mfd/mt6397/rtc.h
index 27883af44f87..001cef6b7302 100644
--- a/include/linux/mfd/mt6397/rtc.h
+++ b/include/linux/mfd/mt6397/rtc.h
@@ -15,8 +15,11 @@
 #include <linux/rtc.h>
 
 #define RTC_BBPU               0x0000
+#define RTC_BBPU_PWREN         BIT(0)
+#define RTC_BBPU_CLR           BIT(1)
+#define RTC_BBPU_RESET_AL      BIT(3)
 #define RTC_BBPU_CBUSY         BIT(6)
-#define RTC_BBPU_KEY            (0x43 << 8)
+#define RTC_BBPU_KEY           (0x43 << 8)
 
 #define RTC_WRTGR_MT6358       0x003a
 #define RTC_WRTGR_MT6397       0x003c
-- 
2.46.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v1 2/2] rtc: mt6397: Add BBPU alarm status reset and shutdown handling
  2025-08-11  8:15 [PATCH v1 0/2] rtc: Enhance RTC driver with BBPU bit definitions and shutdown handling ot_shunxi.zhang
  2025-08-11  8:15 ` [PATCH v1 1/2] mfd: mt6397: Add new bit definitions for RTC_BBPU register ot_shunxi.zhang
@ 2025-08-11  8:15 ` ot_shunxi.zhang
  2025-08-11 13:09   ` Alexandre Belloni
  1 sibling, 1 reply; 11+ messages in thread
From: ot_shunxi.zhang @ 2025-08-11  8:15 UTC (permalink / raw)
  To: Eddie Huang, Sean Wang, Alexandre Belloni, Matthias Brugger,
	Lee Jones, Shunxi Zhang
  Cc: linux-arm-kernel, linux-mediatek, linux-rtc, linux-kernel,
	sirius.wang, vince-wl.liu, jh.hsu

From: Shunxi Zhang <ot_shunxi.zhang@mediatek.com>

This patch introduces a new function, mtk_rtc_reset_bbpu_alarm_status,
to reset the BBPU alarm status in the MT6397 RTC driver. This function
writes the necessary bits to the RTC_BBPU register to clear the alarm
status and ensure proper operation.

Additionally, the mtk_rtc_shutdown function is added to handle RTC
shutdown events. It resets the BBPU alarm status and updates the
RTC_IRQ_EN register to disable the one-shot alarm interrupt,
ensuring a clean shutdown process.

Signed-off-by: Shunxi Zhang <ot_shunxi.zhang@mediatek.com>
---
 drivers/rtc/rtc-mt6397.c | 36 +++++++++++++++++++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
index 692c00ff544b..063bd399de8c 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,8 @@ 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);
+
+		mtk_rtc_reset_bbpu_alarm_status(rtc);
 		mutex_unlock(&rtc->lock);
 
 		return IRQ_HANDLED;
@@ -297,6 +314,22 @@ 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);
+	int ret = 0;
+
+	mtk_rtc_reset_bbpu_alarm_status(rtc);
+
+	ret = regmap_update_bits(rtc->regmap,
+				 rtc->addr_base + RTC_IRQ_EN,
+				 RTC_IRQ_EN_ONESHOT_AL, 0);
+	if (ret < 0)
+		return;
+
+	mtk_rtc_write_trigger(rtc);
+}
+
 #ifdef CONFIG_PM_SLEEP
 static int mt6397_rtc_suspend(struct device *dev)
 {
@@ -345,7 +378,8 @@ static struct platform_driver mtk_rtc_driver = {
 		.of_match_table = mt6397_rtc_of_match,
 		.pm = &mt6397_pm_ops,
 	},
-	.probe	= mtk_rtc_probe,
+	.probe = mtk_rtc_probe,
+	.shutdown = mtk_rtc_shutdown,
 };
 
 module_platform_driver(mtk_rtc_driver);
-- 
2.46.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v1 1/2] mfd: mt6397: Add new bit definitions for RTC_BBPU register
  2025-08-11  8:15 ` [PATCH v1 1/2] mfd: mt6397: Add new bit definitions for RTC_BBPU register ot_shunxi.zhang
@ 2025-08-11 11:02   ` Krzysztof Kozlowski
  2025-08-11 11:21     ` Giorgi Tchankvetadze
  2025-08-19  7:54     ` Shunxi Zhang (章顺喜)
  0 siblings, 2 replies; 11+ messages in thread
From: Krzysztof Kozlowski @ 2025-08-11 11:02 UTC (permalink / raw)
  To: ot_shunxi.zhang, Eddie Huang, Sean Wang, Alexandre Belloni,
	Matthias Brugger, Lee Jones
  Cc: linux-arm-kernel, linux-mediatek, linux-rtc, linux-kernel,
	sirius.wang, vince-wl.liu, jh.hsu

On 11/08/2025 10:15, ot_shunxi.zhang@mediatek.com wrote:
> From: Shunxi Zhang <ot_shunxi.zhang@mediatek.com>
> 
> This patch adds new bit definitions for the RTC_BBPU register in the

Why? There is no user of these. Don't add useless defines.

> mt6397 RTC header file. The following bit definitions are introduced:

Hm?

> 
> Signed-off-by: Shunxi Zhang <ot_shunxi.zhang@mediatek.com>
> ---
>  include/linux/mfd/mt6397/rtc.h | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/mfd/mt6397/rtc.h b/include/linux/mfd/mt6397/rtc.h
> index 27883af44f87..001cef6b7302 100644
> --- a/include/linux/mfd/mt6397/rtc.h
> +++ b/include/linux/mfd/mt6397/rtc.h
> @@ -15,8 +15,11 @@
>  #include <linux/rtc.h>
>  
>  #define RTC_BBPU               0x0000
> +#define RTC_BBPU_PWREN         BIT(0)
> +#define RTC_BBPU_CLR           BIT(1)
> +#define RTC_BBPU_RESET_AL      BIT(3)
>  #define RTC_BBPU_CBUSY         BIT(6)
> -#define RTC_BBPU_KEY            (0x43 << 8)
> +#define RTC_BBPU_KEY           (0x43 << 8)


Why?



Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v1 1/2] mfd: mt6397: Add new bit definitions for RTC_BBPU register
  2025-08-11 11:02   ` Krzysztof Kozlowski
@ 2025-08-11 11:21     ` Giorgi Tchankvetadze
  2025-08-19  7:53       ` Shunxi Zhang (章顺喜)
  2025-08-19  7:54     ` Shunxi Zhang (章顺喜)
  1 sibling, 1 reply; 11+ messages in thread
From: Giorgi Tchankvetadze @ 2025-08-11 11:21 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: ot_shunxi.zhang, Eddie Huang, Sean Wang, Alexandre Belloni,
	Matthias Brugger, Lee Jones, linux-arm-kernel, linux-mediatek,
	linux-rtc, linux-kernel, sirius.wang, vince-wl.liu, jh.hsu

Cc: Krzysztof Kozlowski <krzk@kernel.org>,
    linux-mediatek@lists.infradead.org,
    linux-rtc@vger.kernel.org,
    linux-kernel@vger.kernel.org

Shunxi,

Can you confirm whether `RTC_BBPU_PWREN` (bit 0),
`RTC_BBPU_CLR` (bit 1) and `RTC_BBPU_RESET_AL` (bit 3) are documented
in the MT6397 datasheet (please cite section/page)? They look like
standard RTC controls (power enable, clear/reset, alarm reset) and
might be useful to include, but I agree with Krzysztof that adding
definitions with no users can accumulate technical debt.

Suggestion: either
- add the definitions when a driver actually needs them, or
- keep them now but add a short rationale in the commit message
  (datasheet reference + intended use) so future reviewers understand
  why they exist.

Also: please split cosmetic whitespace fixes (RTC_BBPU_KEY) into a
separate patch to make review/merge easier.

Thanks for the patch; I’m following the thread.

— Giorgi


On Mon, Aug 11, 2025 at 3:03 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 11/08/2025 10:15, ot_shunxi.zhang@mediatek.com wrote:
> > From: Shunxi Zhang <ot_shunxi.zhang@mediatek.com>
> >
> > This patch adds new bit definitions for the RTC_BBPU register in the
>
> Why? There is no user of these. Don't add useless defines.
>
> > mt6397 RTC header file. The following bit definitions are introduced:
>
> Hm?
>
> >
> > Signed-off-by: Shunxi Zhang <ot_shunxi.zhang@mediatek.com>
> > ---
> >  include/linux/mfd/mt6397/rtc.h | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/linux/mfd/mt6397/rtc.h b/include/linux/mfd/mt6397/rtc.h
> > index 27883af44f87..001cef6b7302 100644
> > --- a/include/linux/mfd/mt6397/rtc.h
> > +++ b/include/linux/mfd/mt6397/rtc.h
> > @@ -15,8 +15,11 @@
> >  #include <linux/rtc.h>
> >
> >  #define RTC_BBPU               0x0000
> > +#define RTC_BBPU_PWREN         BIT(0)
> > +#define RTC_BBPU_CLR           BIT(1)
> > +#define RTC_BBPU_RESET_AL      BIT(3)
> >  #define RTC_BBPU_CBUSY         BIT(6)
> > -#define RTC_BBPU_KEY            (0x43 << 8)
> > +#define RTC_BBPU_KEY           (0x43 << 8)
>
>
> Why?
>
>
>
> Best regards,
> Krzysztof
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v1 2/2] rtc: mt6397: Add BBPU alarm status reset and shutdown handling
  2025-08-11  8:15 ` [PATCH v1 2/2] rtc: mt6397: Add BBPU alarm status reset and shutdown handling ot_shunxi.zhang
@ 2025-08-11 13:09   ` Alexandre Belloni
  2025-08-19  7:48     ` Shunxi Zhang (章顺喜)
  0 siblings, 1 reply; 11+ messages in thread
From: Alexandre Belloni @ 2025-08-11 13:09 UTC (permalink / raw)
  To: ot_shunxi.zhang
  Cc: Eddie Huang, Sean Wang, Matthias Brugger, Lee Jones,
	linux-arm-kernel, linux-mediatek, linux-rtc, linux-kernel,
	sirius.wang, vince-wl.liu, jh.hsu

On 11/08/2025 16:15:34+0800, ot_shunxi.zhang@mediatek.com wrote:
> From: Shunxi Zhang <ot_shunxi.zhang@mediatek.com>
> 
> This patch introduces a new function, mtk_rtc_reset_bbpu_alarm_status,
> to reset the BBPU alarm status in the MT6397 RTC driver. This function
> writes the necessary bits to the RTC_BBPU register to clear the alarm
> status and ensure proper operation.
> 
> Additionally, the mtk_rtc_shutdown function is added to handle RTC
> shutdown events. It resets the BBPU alarm status and updates the
> RTC_IRQ_EN register to disable the one-shot alarm interrupt,
> ensuring a clean shutdown process.
> 
> Signed-off-by: Shunxi Zhang <ot_shunxi.zhang@mediatek.com>
> ---
>  drivers/rtc/rtc-mt6397.c | 36 +++++++++++++++++++++++++++++++++++-
>  1 file changed, 35 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
> index 692c00ff544b..063bd399de8c 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,8 @@ 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);
> +
> +		mtk_rtc_reset_bbpu_alarm_status(rtc);
>  		mutex_unlock(&rtc->lock);
>  
>  		return IRQ_HANDLED;
> @@ -297,6 +314,22 @@ 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);
> +	int ret = 0;
> +
> +	mtk_rtc_reset_bbpu_alarm_status(rtc);
> +
> +	ret = regmap_update_bits(rtc->regmap,
> +				 rtc->addr_base + RTC_IRQ_EN,
> +				 RTC_IRQ_EN_ONESHOT_AL, 0);
> +	if (ret < 0)
> +		return;
> +
> +	mtk_rtc_write_trigger(rtc);

The whole goal of the RTC is to wakeup the system, why would you disable
the alarm on shutdown?

> +}
> +
>  #ifdef CONFIG_PM_SLEEP
>  static int mt6397_rtc_suspend(struct device *dev)
>  {
> @@ -345,7 +378,8 @@ static struct platform_driver mtk_rtc_driver = {
>  		.of_match_table = mt6397_rtc_of_match,
>  		.pm = &mt6397_pm_ops,
>  	},
> -	.probe	= mtk_rtc_probe,
> +	.probe = mtk_rtc_probe,
> +	.shutdown = mtk_rtc_shutdown,
>  };
>  
>  module_platform_driver(mtk_rtc_driver);
> -- 
> 2.46.0
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v1 2/2] rtc: mt6397: Add BBPU alarm status reset and shutdown handling
  2025-08-11 13:09   ` Alexandre Belloni
@ 2025-08-19  7:48     ` Shunxi Zhang (章顺喜)
  0 siblings, 0 replies; 11+ messages in thread
From: Shunxi Zhang (章顺喜) @ 2025-08-19  7:48 UTC (permalink / raw)
  To: alexandre.belloni@bootlin.com
  Cc: linux-mediatek@lists.infradead.org,
	Eddie Huang (黃智傑),
	linux-kernel@vger.kernel.org,
	Vince-WL Liu (劉文龍),
	Jh Hsu (許希孜), Sean Wang,
	linux-arm-kernel@lists.infradead.org, lee@kernel.org,
	matthias.bgg@gmail.com, linux-rtc@vger.kernel.org,
	Sirius Wang (王皓昱)

On Mon, 2025-08-11 at 15:09 +0200, Alexandre Belloni wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On 11/08/2025 16:15:34+0800, ot_shunxi.zhang@mediatek.com wrote:
> > From: Shunxi Zhang <ot_shunxi.zhang@mediatek.com>
> > 
> > This patch introduces a new function,
> > mtk_rtc_reset_bbpu_alarm_status,
> > to reset the BBPU alarm status in the MT6397 RTC driver. This
> > function
> > writes the necessary bits to the RTC_BBPU register to clear the
> > alarm
> > status and ensure proper operation.
> > 
> > Additionally, the mtk_rtc_shutdown function is added to handle RTC
> > shutdown events. It resets the BBPU alarm status and updates the
> > RTC_IRQ_EN register to disable the one-shot alarm interrupt,
> > ensuring a clean shutdown process.
> > 
> > Signed-off-by: Shunxi Zhang <ot_shunxi.zhang@mediatek.com>
> > ---
> >  drivers/rtc/rtc-mt6397.c | 36 +++++++++++++++++++++++++++++++++++-
> >  1 file changed, 35 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
> > index 692c00ff544b..063bd399de8c 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,8 @@ 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);
> > +
> > +             mtk_rtc_reset_bbpu_alarm_status(rtc);
> >               mutex_unlock(&rtc->lock);
> > 
> >               return IRQ_HANDLED;
> > @@ -297,6 +314,22 @@ 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);
> > +     int ret = 0;
> > +
> > +     mtk_rtc_reset_bbpu_alarm_status(rtc);
> > +
> > +     ret = regmap_update_bits(rtc->regmap,
> > +                              rtc->addr_base + RTC_IRQ_EN,
> > +                              RTC_IRQ_EN_ONESHOT_AL, 0);
> > +     if (ret < 0)
> > +             return;
> > +
> > +     mtk_rtc_write_trigger(rtc);
> 
> The whole goal of the RTC is to wakeup the system, why would you
> disable
> the alarm on shutdown?

Dear sir,
  I will remove the flow of "disable alarm shutdown"
in next version. thanks for your comments.  

Best regards
shunxi zhang

> 
> > +}
> > +
> >  #ifdef CONFIG_PM_SLEEP
> >  static int mt6397_rtc_suspend(struct device *dev)
> >  {
> > @@ -345,7 +378,8 @@ static struct platform_driver mtk_rtc_driver =
> > {
> >               .of_match_table = mt6397_rtc_of_match,
> >               .pm = &mt6397_pm_ops,
> >       },
> > -     .probe  = mtk_rtc_probe,
> > +     .probe = mtk_rtc_probe,
> > +     .shutdown = mtk_rtc_shutdown,
> >  };
> > 
> >  module_platform_driver(mtk_rtc_driver);
> > --
> > 2.46.0
> > 
> 
> --
> Alexandre Belloni, co-owner and COO, Bootlin
> Embedded Linux and Kernel engineering
> 
https://urldefense.com/v3/__https://bootlin.com__;!!CTRNKA9wMg0ARbw!hGMitEEUU-sbH-rcMCMQ4Vlsn7NQeHdUrg9nEKYPyDME5fkbgsVciZd8SURxkvWA9Z1qX3oyNVZEWvVatYExZ8D7cFvdiyS_3Q_6$

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v1 1/2] mfd: mt6397: Add new bit definitions for RTC_BBPU register
  2025-08-11 11:21     ` Giorgi Tchankvetadze
@ 2025-08-19  7:53       ` Shunxi Zhang (章顺喜)
  0 siblings, 0 replies; 11+ messages in thread
From: Shunxi Zhang (章顺喜) @ 2025-08-19  7:53 UTC (permalink / raw)
  To: giorgitchankvetadze1997@gmail.com, krzk@kernel.org
  Cc: linux-mediatek@lists.infradead.org,
	Eddie Huang (黃智傑),
	alexandre.belloni@bootlin.com, linux-kernel@vger.kernel.org,
	Jh Hsu (許希孜),
	Vince-WL Liu (劉文龍), Sean Wang,
	linux-arm-kernel@lists.infradead.org, lee@kernel.org,
	matthias.bgg@gmail.com, linux-rtc@vger.kernel.org,
	Sirius Wang (王皓昱)

On Mon, 2025-08-11 at 15:21 +0400, Giorgi Tchankvetadze wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> Cc: Krzysztof Kozlowski <krzk@kernel.org>,
>     linux-mediatek@lists.infradead.org,
>     linux-rtc@vger.kernel.org,
>     linux-kernel@vger.kernel.org
> 
> Shunxi,
> 
> Can you confirm whether `RTC_BBPU_PWREN` (bit 0),
> `RTC_BBPU_CLR` (bit 1) and `RTC_BBPU_RESET_AL` (bit 3) are documented
> in the MT6397 datasheet (please cite section/page)? They look like
> standard RTC controls (power enable, clear/reset, alarm reset) and
> might be useful to include, but I agree with Krzysztof that adding
> definitions with no users can accumulate technical debt.
> 
> Suggestion: either
> - add the definitions when a driver actually needs them, or
> - keep them now but add a short rationale in the commit message
>   (datasheet reference + intended use) so future reviewers understand
>   why they exist.
> 
> Also: please split cosmetic whitespace fixes (RTC_BBPU_KEY) into a
> separate patch to make review/merge easier.
> 
> Thanks for the patch; I’m following the thread.
> 
> — Giorgi

Dear sir,
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.

Thanks for your commnets.

Best regards
Shunxi Zhangus
> 
> 
> On Mon, Aug 11, 2025 at 3:03 PM Krzysztof Kozlowski <krzk@kernel.org>
> wrote:
> > 
> > On 11/08/2025 10:15, ot_shunxi.zhang@mediatek.com wrote:
> > > From: Shunxi Zhang <ot_shunxi.zhang@mediatek.com>
> > > 
> > > This patch adds new bit definitions for the RTC_BBPU register in
> > > the
> > 
> > Why? There is no user of these. Don't add useless defines.
> > 
> > > mt6397 RTC header file. The following bit definitions are
> > > introduced:
> > 
> > Hm?
> > 
> > > 
> > > Signed-off-by: Shunxi Zhang <ot_shunxi.zhang@mediatek.com>
> > > ---
> > >  include/linux/mfd/mt6397/rtc.h | 5 ++++-
> > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/include/linux/mfd/mt6397/rtc.h
> > > b/include/linux/mfd/mt6397/rtc.h
> > > index 27883af44f87..001cef6b7302 100644
> > > --- a/include/linux/mfd/mt6397/rtc.h
> > > +++ b/include/linux/mfd/mt6397/rtc.h
> > > @@ -15,8 +15,11 @@
> > >  #include <linux/rtc.h>
> > > 
> > >  #define RTC_BBPU               0x0000
> > > +#define RTC_BBPU_PWREN         BIT(0)
> > > +#define RTC_BBPU_CLR           BIT(1)
> > > +#define RTC_BBPU_RESET_AL      BIT(3)
> > >  #define RTC_BBPU_CBUSY         BIT(6)
> > > -#define RTC_BBPU_KEY            (0x43 << 8)
> > > +#define RTC_BBPU_KEY           (0x43 << 8)
> > 
> > 
> > Why?
> > 
> > 
> > 
> > Best regards,
> > Krzysztof
> > 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v1 1/2] mfd: mt6397: Add new bit definitions for RTC_BBPU register
  2025-08-11 11:02   ` Krzysztof Kozlowski
  2025-08-11 11:21     ` Giorgi Tchankvetadze
@ 2025-08-19  7:54     ` Shunxi Zhang (章顺喜)
  2025-08-19  8:16       ` Krzysztof Kozlowski
  1 sibling, 1 reply; 11+ messages in thread
From: Shunxi Zhang (章顺喜) @ 2025-08-19  7:54 UTC (permalink / raw)
  To: Sean Wang, matthias.bgg@gmail.com, lee@kernel.org,
	alexandre.belloni@bootlin.com, krzk@kernel.org,
	Eddie Huang (黃智傑)
  Cc: linux-arm-kernel@lists.infradead.org, linux-rtc@vger.kernel.org,
	linux-mediatek@lists.infradead.org,
	Sirius Wang (王皓昱),
	linux-kernel@vger.kernel.org, Jh Hsu (許希孜),
	Vince-WL Liu (劉文龍)

On Mon, 2025-08-11 at 13:02 +0200, Krzysztof Kozlowski wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On 11/08/2025 10:15, ot_shunxi.zhang@mediatek.com wrote:
> > From: Shunxi Zhang <ot_shunxi.zhang@mediatek.com>
> > 
> > This patch adds new bit definitions for the RTC_BBPU register in
> > the
> 
> Why? There is no user of these. Don't add useless defines.
> 
> > mt6397 RTC header file. The following bit definitions are
> > introduced:
> 
> Hm?
> 
> > 
> > Signed-off-by: Shunxi Zhang <ot_shunxi.zhang@mediatek.com>
> > ---
> >  include/linux/mfd/mt6397/rtc.h | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/include/linux/mfd/mt6397/rtc.h
> > b/include/linux/mfd/mt6397/rtc.h
> > index 27883af44f87..001cef6b7302 100644
> > --- a/include/linux/mfd/mt6397/rtc.h
> > +++ b/include/linux/mfd/mt6397/rtc.h
> > @@ -15,8 +15,11 @@
> >  #include <linux/rtc.h>
> > 
> >  #define RTC_BBPU               0x0000
> > +#define RTC_BBPU_PWREN         BIT(0)
> > +#define RTC_BBPU_CLR           BIT(1)
> > +#define RTC_BBPU_RESET_AL      BIT(3)
> >  #define RTC_BBPU_CBUSY         BIT(6)
> > -#define RTC_BBPU_KEY            (0x43 << 8)
> > +#define RTC_BBPU_KEY           (0x43 << 8)
> 
> 
> Why?
> 
> 
> 
> Best regards,
> Krzysztof

Dear sir,
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.

Thanks for your commnets.

Best regards
Shunxi Zhangus


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v1 1/2] mfd: mt6397: Add new bit definitions for RTC_BBPU register
  2025-08-19  7:54     ` Shunxi Zhang (章顺喜)
@ 2025-08-19  8:16       ` Krzysztof Kozlowski
  2025-08-19  8:32         ` Shunxi Zhang (章顺喜)
  0 siblings, 1 reply; 11+ messages in thread
From: Krzysztof Kozlowski @ 2025-08-19  8:16 UTC (permalink / raw)
  To: Shunxi Zhang (章顺喜), Sean Wang,
	matthias.bgg@gmail.com, lee@kernel.org,
	alexandre.belloni@bootlin.com,
	Eddie Huang (黃智傑)
  Cc: linux-arm-kernel@lists.infradead.org, linux-rtc@vger.kernel.org,
	linux-mediatek@lists.infradead.org,
	Sirius Wang (王皓昱),
	linux-kernel@vger.kernel.org, Jh Hsu (許希孜),
	Vince-WL Liu (劉文龍)

On 19/08/2025 09:54, Shunxi Zhang (章顺喜) wrote:
>>>  #define RTC_BBPU               0x0000
>>> +#define RTC_BBPU_PWREN         BIT(0)
>>> +#define RTC_BBPU_CLR           BIT(1)
>>> +#define RTC_BBPU_RESET_AL      BIT(3)
>>>  #define RTC_BBPU_CBUSY         BIT(6)
>>> -#define RTC_BBPU_KEY            (0x43 << 8)
>>> +#define RTC_BBPU_KEY           (0x43 << 8)
>>
>>
>> Why?
>>
>>
>>
>> Best regards,
>> Krzysztof
> 
> Dear sir,
> 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.


I understand nothing from this reply. I do not see it related to my
questions at all. Do you want to say that you change indentation,
because MT6397 is integration? That makes absolutely NO SENSE!

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v1 1/2] mfd: mt6397: Add new bit definitions for RTC_BBPU register
  2025-08-19  8:16       ` Krzysztof Kozlowski
@ 2025-08-19  8:32         ` Shunxi Zhang (章顺喜)
  0 siblings, 0 replies; 11+ messages in thread
From: Shunxi Zhang (章顺喜) @ 2025-08-19  8:32 UTC (permalink / raw)
  To: Sean Wang, matthias.bgg@gmail.com, lee@kernel.org,
	Eddie Huang (黃智傑), krzk@kernel.org,
	alexandre.belloni@bootlin.com
  Cc: linux-arm-kernel@lists.infradead.org, linux-rtc@vger.kernel.org,
	linux-mediatek@lists.infradead.org,
	Sirius Wang (王皓昱),
	linux-kernel@vger.kernel.org, Jh Hsu (許希孜),
	Vince-WL Liu (劉文龍)

On Tue, 2025-08-19 at 10:16 +0200, Krzysztof Kozlowski wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On 19/08/2025 09:54, Shunxi Zhang (章顺喜) wrote:
> > > >  #define RTC_BBPU               0x0000
> > > > +#define RTC_BBPU_PWREN         BIT(0)
> > > > +#define RTC_BBPU_CLR           BIT(1)
> > > > +#define RTC_BBPU_RESET_AL      BIT(3)
> > > >  #define RTC_BBPU_CBUSY         BIT(6)
> > > > -#define RTC_BBPU_KEY            (0x43 << 8)
> > > > +#define RTC_BBPU_KEY           (0x43 << 8)
> > > 
> > > 
> > > Why?
> > > 
> > > 
> > > 
> > > Best regards,
> > > Krzysztof
> > 
> > Dear sir,
> > 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.
> 
> 
> I understand nothing from this reply. I do not see it related to my
> questions at all. Do you want to say that you change indentation,
> because MT6397 is integration? That makes absolutely NO SENSE!
> 
> Best regards,
> Krzysztof

Dear sir,
   I will submit a separate patch for the format modification. The
reply to the previous email was not about this format issue. I am very
sorry for causing your misunderstanding.

Best Regards
Shunxi Zhang

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2025-08-19  8:32 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-11  8:15 [PATCH v1 0/2] rtc: Enhance RTC driver with BBPU bit definitions and shutdown handling ot_shunxi.zhang
2025-08-11  8:15 ` [PATCH v1 1/2] mfd: mt6397: Add new bit definitions for RTC_BBPU register ot_shunxi.zhang
2025-08-11 11:02   ` Krzysztof Kozlowski
2025-08-11 11:21     ` Giorgi Tchankvetadze
2025-08-19  7:53       ` Shunxi Zhang (章顺喜)
2025-08-19  7:54     ` Shunxi Zhang (章顺喜)
2025-08-19  8:16       ` Krzysztof Kozlowski
2025-08-19  8:32         ` Shunxi Zhang (章顺喜)
2025-08-11  8:15 ` [PATCH v1 2/2] rtc: mt6397: Add BBPU alarm status reset and shutdown handling ot_shunxi.zhang
2025-08-11 13:09   ` Alexandre Belloni
2025-08-19  7:48     ` Shunxi Zhang (章顺喜)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).