[PATCH 0/6] Add DA9062 PMIC and built-in RTC support for RZ/G2UL SMARC EVK

[PATCH 0/6] Add DA9062 PMIC and built-in RTC support for RZ/G2UL SMARC EVK

[Biju Das]

The patch series aims to add support for DA9062 PMIC and built-in RTC found
on RZ/{G2UL,Five} SMARC EVK. On these platforms there is no IRQ populated
by default. Add irq optional support and convert the bindings to
json-schema.

Biju Das (6):
  rtc: da9063: Make IRQ as optional
  rtc: da9063: Use device_get_match_data()
  rtc: da9063: Use dev_err_probe()
  dt-bindings: mfd: Convert da9062 to json-schema
  arm64: dts: renesas: rzg2ul-smarc: Enable PMIC and built-in RTC
  arm64: defconfig: Enable Renesas DA9062 defconfig

 .../devicetree/bindings/mfd/da9062.txt        | 124 ----------
 .../devicetree/bindings/mfd/dlg,da9062.yaml   | 220 ++++++++++++++++++
 arch/arm64/boot/dts/renesas/rzg2ul-smarc.dtsi |  29 +++
 arch/arm64/configs/defconfig                  |   2 +
 drivers/rtc/rtc-da9063.c                      |  88 ++++---
 5 files changed, 290 insertions(+), 173 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/mfd/da9062.txt
 create mode 100644 Documentation/devicetree/bindings/mfd/dlg,da9062.yaml

-- 
2.25.1

[PATCH 1/6] rtc: da9063: Make IRQ as optional

[Biju Das]

On some platforms (eg: RZ/{G2UL,Five} SMARC EVK), there is no IRQ
populated by default. Add irq optional support.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/rtc/rtc-da9063.c | 39 ++++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/drivers/rtc/rtc-da9063.c b/drivers/rtc/rtc-da9063.c
index 2f5d60622564..613ba2c5d757 100644
--- a/drivers/rtc/rtc-da9063.c
+++ b/drivers/rtc/rtc-da9063.c
@@ -485,25 +485,26 @@ static int da9063_rtc_probe(struct platform_device *pdev)
 		clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, rtc->rtc_dev->features);
 	}
 
-	irq_alarm = platform_get_irq_byname(pdev, "ALARM");
-	if (irq_alarm < 0)
-		return irq_alarm;
-
-	ret = devm_request_threaded_irq(&pdev->dev, irq_alarm, NULL,
-					da9063_alarm_event,
-					IRQF_TRIGGER_LOW | IRQF_ONESHOT,
-					"ALARM", rtc);
-	if (ret)
-		dev_err(&pdev->dev, "Failed to request ALARM IRQ %d: %d\n",
-			irq_alarm, ret);
-
-	ret = dev_pm_set_wake_irq(&pdev->dev, irq_alarm);
-	if (ret)
-		dev_warn(&pdev->dev,
-			 "Failed to set IRQ %d as a wake IRQ: %d\n",
-			 irq_alarm, ret);
-
-	device_init_wakeup(&pdev->dev, true);
+	irq_alarm = platform_get_irq_byname_optional(pdev, "ALARM");
+	if (irq_alarm >= 0) {
+		ret = devm_request_threaded_irq(&pdev->dev, irq_alarm, NULL,
+						da9063_alarm_event,
+						IRQF_TRIGGER_LOW | IRQF_ONESHOT,
+						"ALARM", rtc);
+		if (ret)
+			dev_err(&pdev->dev, "Failed to request ALARM IRQ %d: %d\n",
+				irq_alarm, ret);
+
+		ret = dev_pm_set_wake_irq(&pdev->dev, irq_alarm);
+		if (ret)
+			dev_warn(&pdev->dev,
+				 "Failed to set IRQ %d as a wake IRQ: %d\n",
+				 irq_alarm, ret);
+
+		device_init_wakeup(&pdev->dev, true);
+	} else {
+		clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, rtc->rtc_dev->features);
+	}
 
 	return devm_rtc_register_device(rtc->rtc_dev);
 }
-- 
2.25.1

Re: [PATCH 1/6] rtc: da9063: Make IRQ as optional

[Geert Uytterhoeven]

Hi Biju,

On Fri, Dec 1, 2023 at 12:08 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> On some platforms (eg: RZ/{G2UL,Five} SMARC EVK), there is no IRQ
> populated by default. Add irq optional support.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Thanks for your patch!

> --- a/drivers/rtc/rtc-da9063.c
> +++ b/drivers/rtc/rtc-da9063.c
> @@ -485,25 +485,26 @@ static int da9063_rtc_probe(struct platform_device *pdev)
>                 clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, rtc->rtc_dev->features);
>         }
>
> -       irq_alarm = platform_get_irq_byname(pdev, "ALARM");
> -       if (irq_alarm < 0)
> -               return irq_alarm;
> -
> -       ret = devm_request_threaded_irq(&pdev->dev, irq_alarm, NULL,
> -                                       da9063_alarm_event,
> -                                       IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> -                                       "ALARM", rtc);
> -       if (ret)
> -               dev_err(&pdev->dev, "Failed to request ALARM IRQ %d: %d\n",
> -                       irq_alarm, ret);
> -
> -       ret = dev_pm_set_wake_irq(&pdev->dev, irq_alarm);
> -       if (ret)
> -               dev_warn(&pdev->dev,
> -                        "Failed to set IRQ %d as a wake IRQ: %d\n",
> -                        irq_alarm, ret);
> -
> -       device_init_wakeup(&pdev->dev, true);
> +       irq_alarm = platform_get_irq_byname_optional(pdev, "ALARM");
> +       if (irq_alarm >= 0) {
> +               ret = devm_request_threaded_irq(&pdev->dev, irq_alarm, NULL,
> +                                               da9063_alarm_event,
> +                                               IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> +                                               "ALARM", rtc);
> +               if (ret)
> +                       dev_err(&pdev->dev, "Failed to request ALARM IRQ %d: %d\n",
> +                               irq_alarm, ret);
> +
> +               ret = dev_pm_set_wake_irq(&pdev->dev, irq_alarm);
> +               if (ret)
> +                       dev_warn(&pdev->dev,
> +                                "Failed to set IRQ %d as a wake IRQ: %d\n",
> +                                irq_alarm, ret);
> +
> +               device_init_wakeup(&pdev->dev, true);
> +       } else {
> +               clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, rtc->rtc_dev->features);

This does not handle and propagate real errors (e.g. -EPROBE_DEFER).

    } else if (irq_alarm != -ENXIO) {
            return irq_alarm;
    } else {
            ....
    }

(I think -ENXIO is the correct error to check for,
 platform_get_irq_byname_optional() really should start returning
 zero for not found)

> +       }
>
>         return devm_rtc_register_device(rtc->rtc_dev);

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH 1/6] rtc: da9063: Make IRQ as optional

[Biju Das]

Hi Geert Uytterhoeven,

Thanks for the feedback.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Subject: Re: [PATCH 1/6] rtc: da9063: Make IRQ as optional
> 
> Hi Biju,
> 
> On Fri, Dec 1, 2023 at 12:08 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > On some platforms (eg: RZ/{G2UL,Five} SMARC EVK), there is no IRQ
> > populated by default. Add irq optional support.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Thanks for your patch!
> 
> > --- a/drivers/rtc/rtc-da9063.c
> > +++ b/drivers/rtc/rtc-da9063.c
> > @@ -485,25 +485,26 @@ static int da9063_rtc_probe(struct platform_device
> *pdev)
> >                 clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, rtc->rtc_dev-
> >features);
> >         }
> >
> > -       irq_alarm = platform_get_irq_byname(pdev, "ALARM");
> > -       if (irq_alarm < 0)
> > -               return irq_alarm;
> > -
> > -       ret = devm_request_threaded_irq(&pdev->dev, irq_alarm, NULL,
> > -                                       da9063_alarm_event,
> > -                                       IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> > -                                       "ALARM", rtc);
> > -       if (ret)
> > -               dev_err(&pdev->dev, "Failed to request ALARM
> IRQ %d: %d\n",
> > -                       irq_alarm, ret);
> > -
> > -       ret = dev_pm_set_wake_irq(&pdev->dev, irq_alarm);
> > -       if (ret)
> > -               dev_warn(&pdev->dev,
> > -                        "Failed to set IRQ %d as a wake IRQ: %d\n",
> > -                        irq_alarm, ret);
> > -
> > -       device_init_wakeup(&pdev->dev, true);
> > +       irq_alarm = platform_get_irq_byname_optional(pdev, "ALARM");
> > +       if (irq_alarm >= 0) {
> > +               ret = devm_request_threaded_irq(&pdev->dev, irq_alarm,
> NULL,
> > +                                               da9063_alarm_event,
> > +                                               IRQF_TRIGGER_LOW |
> IRQF_ONESHOT,
> > +                                               "ALARM", rtc);
> > +               if (ret)
> > +                       dev_err(&pdev->dev, "Failed to request ALARM
> IRQ %d: %d\n",
> > +                               irq_alarm, ret);
> > +
> > +               ret = dev_pm_set_wake_irq(&pdev->dev, irq_alarm);
> > +               if (ret)
> > +                       dev_warn(&pdev->dev,
> > +                                "Failed to set IRQ %d as a wake
> IRQ: %d\n",
> > +                                irq_alarm, ret);
> > +
> > +               device_init_wakeup(&pdev->dev, true);
> > +       } else {
> > +               clear_bit(RTC_FEATURE_UPDATE_INTERRUPT,
> > + rtc->rtc_dev->features);
> 
> This does not handle and propagate real errors (e.g. -EPROBE_DEFER).


Oops. Missed propagating errors.

> 
>     } else if (irq_alarm != -ENXIO) {
>             return irq_alarm;
>     } else {
>             ....
>     }
> 
> (I think -ENXIO is the correct error to check for,
>  platform_get_irq_byname_optional() really should start returning  zero
> for not found)

Agreed, currently, if it is -ENXIO, then not found condition.

Cheers,
Biju 

> 
> > +       }
> >
> >         return devm_rtc_register_device(rtc->rtc_dev);
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-
> m68k.org
> 
> In personal conversations with technical people, I call myself a hacker.
> But when I'm talking to journalists I just say "programmer" or something
> like that.
>                                 -- Linus Torvalds

[PATCH 2/6] rtc: da9063: Use device_get_match_data()

[Biju Das]

Replace of_match_node()->device_get_match_data() for
the data associated with device match.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/rtc/rtc-da9063.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/rtc/rtc-da9063.c b/drivers/rtc/rtc-da9063.c
index 613ba2c5d757..a1ee0705ca88 100644
--- a/drivers/rtc/rtc-da9063.c
+++ b/drivers/rtc/rtc-da9063.c
@@ -377,7 +377,6 @@ static int da9063_rtc_probe(struct platform_device *pdev)
 {
 	struct da9063_compatible_rtc *rtc;
 	const struct da9063_compatible_rtc_regmap *config;
-	const struct of_device_id *match;
 	int irq_alarm;
 	u8 data[RTC_DATA_LEN];
 	int ret;
@@ -385,14 +384,11 @@ static int da9063_rtc_probe(struct platform_device *pdev)
 	if (!pdev->dev.of_node)
 		return -ENXIO;
 
-	match = of_match_node(da9063_compatible_reg_id_table,
-			      pdev->dev.of_node);
-
 	rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
 	if (!rtc)
 		return -ENOMEM;
 
-	rtc->config = match->data;
+	rtc->config = device_get_match_data(&pdev->dev);
 	if (of_device_is_compatible(pdev->dev.of_node, "dlg,da9063-rtc")) {
 		struct da9063 *chip = dev_get_drvdata(pdev->dev.parent);
 
-- 
2.25.1

Re: [PATCH 2/6] rtc: da9063: Use device_get_match_data()

[Geert Uytterhoeven]

On Fri, Dec 1, 2023 at 12:08 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> Replace of_match_node()->device_get_match_data() for
> the data associated with device match.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH 3/6] rtc: da9063: Use dev_err_probe()

[Biju Das]

Replace dev_err()->dev_err_probe() to simpilfy probe().

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/rtc/rtc-da9063.c | 47 +++++++++++++++++-----------------------
 1 file changed, 20 insertions(+), 27 deletions(-)

diff --git a/drivers/rtc/rtc-da9063.c b/drivers/rtc/rtc-da9063.c
index a1ee0705ca88..4d7664340091 100644
--- a/drivers/rtc/rtc-da9063.c
+++ b/drivers/rtc/rtc-da9063.c
@@ -407,57 +407,49 @@ static int da9063_rtc_probe(struct platform_device *pdev)
 				 config->rtc_enable_reg,
 				 config->rtc_enable_mask,
 				 config->rtc_enable_mask);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "Failed to enable RTC\n");
-		return ret;
-	}
+	if (ret < 0)
+		return dev_err_probe(&pdev->dev, ret, "Failed to enable RTC\n");
 
 	ret = regmap_update_bits(rtc->regmap,
 				 config->rtc_enable_32k_crystal_reg,
 				 config->rtc_crystal_mask,
 				 config->rtc_crystal_mask);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "Failed to run 32kHz oscillator\n");
-		return ret;
-	}
+	if (ret < 0)
+		return dev_err_probe(&pdev->dev, ret,
+				     "Failed to run 32kHz oscillator\n");
 
 	ret = regmap_update_bits(rtc->regmap,
 				 config->rtc_alarm_secs_reg,
 				 config->rtc_alarm_status_mask,
 				 0);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "Failed to access RTC alarm register\n");
-		return ret;
-	}
+	if (ret < 0)
+		return dev_err_probe(&pdev->dev, ret,
+				     "Failed to access RTC alarm register\n");
 
 	ret = regmap_update_bits(rtc->regmap,
 				 config->rtc_alarm_secs_reg,
 				 DA9063_ALARM_STATUS_ALARM,
 				 DA9063_ALARM_STATUS_ALARM);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "Failed to access RTC alarm register\n");
-		return ret;
-	}
+	if (ret < 0)
+		return dev_err_probe(&pdev->dev, ret,
+				     "Failed to access RTC alarm register\n");
 
 	ret = regmap_update_bits(rtc->regmap,
 				 config->rtc_alarm_year_reg,
 				 config->rtc_tick_on_mask,
 				 0);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "Failed to disable TICKs\n");
-		return ret;
-	}
+	if (ret < 0)
+		return dev_err_probe(&pdev->dev, ret,
+				     "Failed to disable TICKs\n");
 
 	data[RTC_SEC] = 0;
 	ret = regmap_bulk_read(rtc->regmap,
 			       config->rtc_alarm_secs_reg,
 			       &data[config->rtc_data_start],
 			       config->rtc_alarm_len);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "Failed to read initial alarm data: %d\n",
-			ret);
-		return ret;
-	}
+	if (ret < 0)
+		return dev_err_probe(&pdev->dev, ret,
+				     "Failed to read initial alarm data\n");
 
 	platform_set_drvdata(pdev, rtc);
 
@@ -488,8 +480,9 @@ static int da9063_rtc_probe(struct platform_device *pdev)
 						IRQF_TRIGGER_LOW | IRQF_ONESHOT,
 						"ALARM", rtc);
 		if (ret)
-			dev_err(&pdev->dev, "Failed to request ALARM IRQ %d: %d\n",
-				irq_alarm, ret);
+			return dev_err_probe(&pdev->dev, ret,
+					     "Failed to request ALARM IRQ %d\n",
+					     irq_alarm);
 
 		ret = dev_pm_set_wake_irq(&pdev->dev, irq_alarm);
 		if (ret)
-- 
2.25.1

Re: [PATCH 3/6] rtc: da9063: Use dev_err_probe()

[Geert Uytterhoeven]

Hi Biju,

On Fri, Dec 1, 2023 at 12:08 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> Replace dev_err()->dev_err_probe() to simpilfy probe().
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Thanks for your patch!

> --- a/drivers/rtc/rtc-da9063.c
> +++ b/drivers/rtc/rtc-da9063.c
> @@ -488,8 +480,9 @@ static int da9063_rtc_probe(struct platform_device *pdev)
>                                                 IRQF_TRIGGER_LOW | IRQF_ONESHOT,
>                                                 "ALARM", rtc);
>                 if (ret)
> -                       dev_err(&pdev->dev, "Failed to request ALARM IRQ %d: %d\n",
> -                               irq_alarm, ret);
> +                       return dev_err_probe(&pdev->dev, ret,
> +                                            "Failed to request ALARM IRQ %d\n",
> +                                            irq_alarm);

This changes behavior: before, this was not considered fatal.

>
>                 ret = dev_pm_set_wake_irq(&pdev->dev, irq_alarm);
>                 if (ret)

The rest LGTM, so with the above fixed/clarified:
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH 3/6] rtc: da9063: Use dev_err_probe()

[Biju Das]

Hi Geert,

Thanks for the feedback.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Subject: Re: [PATCH 3/6] rtc: da9063: Use dev_err_probe()
> 
> Hi Biju,
> 
> On Fri, Dec 1, 2023 at 12:08 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > Replace dev_err()->dev_err_probe() to simpilfy probe().
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Thanks for your patch!
> 
> > --- a/drivers/rtc/rtc-da9063.c
> > +++ b/drivers/rtc/rtc-da9063.c
> > @@ -488,8 +480,9 @@ static int da9063_rtc_probe(struct platform_device
> *pdev)
> >                                                 IRQF_TRIGGER_LOW |
> IRQF_ONESHOT,
> >                                                 "ALARM", rtc);
> >                 if (ret)
> > -                       dev_err(&pdev->dev, "Failed to request ALARM
> IRQ %d: %d\n",
> > -                               irq_alarm, ret);
> > +                       return dev_err_probe(&pdev->dev, ret,
> > +                                            "Failed to request ALARM
> IRQ %d\n",
> > +                                            irq_alarm);
> 
> This changes behavior: before, this was not considered fatal.

Agreed. Maybe a separate patch?

if there is no irqhandler on platform with IRQ populated nothing will work,
RTC won't work as "rtc_update_irq " updated in irq handler.
I think it is a fatal condition.

Cheers,
Biju

> 
> >
> >                 ret = dev_pm_set_wake_irq(&pdev->dev, irq_alarm);
> >                 if (ret)
> 
> The rest LGTM, so with the above fixed/clarified:
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-
> m68k.org
> 
> In personal conversations with technical people, I call myself a hacker.
> But when I'm talking to journalists I just say "programmer" or something
> like that.
>                                 -- Linus Torvalds

Re: [PATCH 3/6] rtc: da9063: Use dev_err_probe()

[Alexandre Belloni]

On 01/12/2023 13:30:05+0000, Biju Das wrote:
> Hi Geert,
> 
> Thanks for the feedback.
> 
> > -----Original Message-----
> > From: Geert Uytterhoeven <geert@linux-m68k.org>
> > Subject: Re: [PATCH 3/6] rtc: da9063: Use dev_err_probe()
> > 
> > Hi Biju,
> > 
> > On Fri, Dec 1, 2023 at 12:08 PM Biju Das <biju.das.jz@bp.renesas.com>
> > wrote:
> > > Replace dev_err()->dev_err_probe() to simpilfy probe().
> > >
> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > 
> > Thanks for your patch!
> > 
> > > --- a/drivers/rtc/rtc-da9063.c
> > > +++ b/drivers/rtc/rtc-da9063.c
> > > @@ -488,8 +480,9 @@ static int da9063_rtc_probe(struct platform_device
> > *pdev)
> > >                                                 IRQF_TRIGGER_LOW |
> > IRQF_ONESHOT,
> > >                                                 "ALARM", rtc);
> > >                 if (ret)
> > > -                       dev_err(&pdev->dev, "Failed to request ALARM
> > IRQ %d: %d\n",
> > > -                               irq_alarm, ret);
> > > +                       return dev_err_probe(&pdev->dev, ret,
> > > +                                            "Failed to request ALARM
> > IRQ %d\n",
> > > +                                            irq_alarm);
> > 
> > This changes behavior: before, this was not considered fatal.
> 
> Agreed. Maybe a separate patch?
> 
> if there is no irqhandler on platform with IRQ populated nothing will work,
> RTC won't work as "rtc_update_irq " updated in irq handler.
> I think it is a fatal condition.

This is not true, an RTC can wake up a system without an IRQ.

> 
> Cheers,
> Biju
> 
> > 
> > >
> > >                 ret = dev_pm_set_wake_irq(&pdev->dev, irq_alarm);
> > >                 if (ret)
> > 
> > The rest LGTM, so with the above fixed/clarified:
> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > 
> > Gr{oetje,eeting}s,
> > 
> >                         Geert
> > 
> > --
> > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-
> > m68k.org
> > 
> > In personal conversations with technical people, I call myself a hacker.
> > But when I'm talking to journalists I just say "programmer" or something
> > like that.
> >                                 -- Linus Torvalds

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

RE: [PATCH 3/6] rtc: da9063: Use dev_err_probe()

[Biju Das]

Hi Alexandre Belloni,

Thanks for the feedback.

> -----Original Message-----
> Subject: Re: [PATCH 3/6] rtc: da9063: Use dev_err_probe()
> 
> On 01/12/2023 13:30:05+0000, Biju Das wrote:
> > Hi Geert,
> >
> > Thanks for the feedback.
> >
> > > -----Original Message-----
> > > From: Geert Uytterhoeven <geert@linux-m68k.org>
> > > Subject: Re: [PATCH 3/6] rtc: da9063: Use dev_err_probe()
> > >
> > > Hi Biju,
> > >
> > > On Fri, Dec 1, 2023 at 12:08 PM Biju Das
> > > <biju.das.jz@bp.renesas.com>
> > > wrote:
> > > > Replace dev_err()->dev_err_probe() to simpilfy probe().
> > > >
> > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > >
> > > Thanks for your patch!
> > >
> > > > --- a/drivers/rtc/rtc-da9063.c
> > > > +++ b/drivers/rtc/rtc-da9063.c
> > > > @@ -488,8 +480,9 @@ static int da9063_rtc_probe(struct
> > > > platform_device
> > > *pdev)
> > > >                                                 IRQF_TRIGGER_LOW |
> > > IRQF_ONESHOT,
> > > >                                                 "ALARM", rtc);
> > > >                 if (ret)
> > > > -                       dev_err(&pdev->dev, "Failed to request ALARM
> > > IRQ %d: %d\n",
> > > > -                               irq_alarm, ret);
> > > > +                       return dev_err_probe(&pdev->dev, ret,
> > > > +                                            "Failed to request
> > > > + ALARM
> > > IRQ %d\n",
> > > > +                                            irq_alarm);
> > >
> > > This changes behavior: before, this was not considered fatal.
> >
> > Agreed. Maybe a separate patch?
> >
> > if there is no irqhandler on platform with IRQ populated nothing will
> > work, RTC won't work as "rtc_update_irq " updated in irq handler.
> > I think it is a fatal condition.
> 
> This is not true, an RTC can wake up a system without an IRQ.

Agreed, I will restore the behaviour for this case.
It may not be fatal. But basic "hwclock -r" from userspace won't
work as " RTC_FEATURE_UPDATE_INTERRUPT" set and there is no irqhandler.

Cheers,
Biju



Cheers,
Biju

> 
> >
> > Cheers,
> > Biju
> >
> > >
> > > >
> > > >                 ret = dev_pm_set_wake_irq(&pdev->dev, irq_alarm);
> > > >                 if (ret)
> > >
> > > The rest LGTM, so with the above fixed/clarified:
> > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > >
> > > Gr{oetje,eeting}s,
> > >
> > >                         Geert
> > >
> > > --
> > > Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> > > geert@linux- m68k.org
> > >
> > > In personal conversations with technical people, I call myself a
> hacker.
> > > But when I'm talking to journalists I just say "programmer" or
> > > something like that.
> > >                                 -- Linus Torvalds
> 
> --
> Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and Kernel
> engineering
> https://jpn01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbootlin.
> com%2F&data=05%7C01%7Cbiju.das.jz%40bp.renesas.com%7Cb699f48656d34a923a640
> 8dbf28104af%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C63837040817604431
> 5%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1h
> aWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=E9tDi08sBRoh4tBccQB%2B8az%2BqQ4%2
> FtQOpFjdPgU8zQXc%3D&reserved=0

Re: [PATCH 3/6] rtc: da9063: Use dev_err_probe()

[Alexandre Belloni]

On 01/12/2023 15:43:27+0000, Biju Das wrote:
> Hi Alexandre Belloni,
> 
> Thanks for the feedback.
> 
> > -----Original Message-----
> > Subject: Re: [PATCH 3/6] rtc: da9063: Use dev_err_probe()
> > 
> > On 01/12/2023 13:30:05+0000, Biju Das wrote:
> > > Hi Geert,
> > >
> > > Thanks for the feedback.
> > >
> > > > -----Original Message-----
> > > > From: Geert Uytterhoeven <geert@linux-m68k.org>
> > > > Subject: Re: [PATCH 3/6] rtc: da9063: Use dev_err_probe()
> > > >
> > > > Hi Biju,
> > > >
> > > > On Fri, Dec 1, 2023 at 12:08 PM Biju Das
> > > > <biju.das.jz@bp.renesas.com>
> > > > wrote:
> > > > > Replace dev_err()->dev_err_probe() to simpilfy probe().
> > > > >
> > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > >
> > > > Thanks for your patch!
> > > >
> > > > > --- a/drivers/rtc/rtc-da9063.c
> > > > > +++ b/drivers/rtc/rtc-da9063.c
> > > > > @@ -488,8 +480,9 @@ static int da9063_rtc_probe(struct
> > > > > platform_device
> > > > *pdev)
> > > > >                                                 IRQF_TRIGGER_LOW |
> > > > IRQF_ONESHOT,
> > > > >                                                 "ALARM", rtc);
> > > > >                 if (ret)
> > > > > -                       dev_err(&pdev->dev, "Failed to request ALARM
> > > > IRQ %d: %d\n",
> > > > > -                               irq_alarm, ret);
> > > > > +                       return dev_err_probe(&pdev->dev, ret,
> > > > > +                                            "Failed to request
> > > > > + ALARM
> > > > IRQ %d\n",
> > > > > +                                            irq_alarm);
> > > >
> > > > This changes behavior: before, this was not considered fatal.
> > >
> > > Agreed. Maybe a separate patch?
> > >
> > > if there is no irqhandler on platform with IRQ populated nothing will
> > > work, RTC won't work as "rtc_update_irq " updated in irq handler.
> > > I think it is a fatal condition.
> > 
> > This is not true, an RTC can wake up a system without an IRQ.
> 
> Agreed, I will restore the behaviour for this case.
> It may not be fatal. But basic "hwclock -r" from userspace won't
> work as " RTC_FEATURE_UPDATE_INTERRUPT" set and there is no irqhandler.
> 

RTC_FEATURE_ALARM is what you should clear and you have to fallback to
the irq is not present case.

> Cheers,
> Biju
> 
> 
> 
> Cheers,
> Biju
> 
> > 
> > >
> > > Cheers,
> > > Biju
> > >
> > > >
> > > > >
> > > > >                 ret = dev_pm_set_wake_irq(&pdev->dev, irq_alarm);
> > > > >                 if (ret)
> > > >
> > > > The rest LGTM, so with the above fixed/clarified:
> > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > >
> > > > Gr{oetje,eeting}s,
> > > >
> > > >                         Geert
> > > >
> > > > --
> > > > Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> > > > geert@linux- m68k.org
> > > >
> > > > In personal conversations with technical people, I call myself a
> > hacker.
> > > > But when I'm talking to journalists I just say "programmer" or
> > > > something like that.
> > > >                                 -- Linus Torvalds
> > 
> > --
> > Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and Kernel
> > engineering
> > https://jpn01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbootlin.
> > com%2F&data=05%7C01%7Cbiju.das.jz%40bp.renesas.com%7Cb699f48656d34a923a640
> > 8dbf28104af%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C63837040817604431
> > 5%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1h
> > aWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=E9tDi08sBRoh4tBccQB%2B8az%2BqQ4%2
> > FtQOpFjdPgU8zQXc%3D&reserved=0

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

RE: [PATCH 3/6] rtc: da9063: Use dev_err_probe()

[Biju Das]

Hi Alexandre Belloni,

> -----Original Message-----
> From: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Subject: Re: [PATCH 3/6] rtc: da9063: Use dev_err_probe()
> 
> On 01/12/2023 15:43:27+0000, Biju Das wrote:
> > Hi Alexandre Belloni,
> >
> > Thanks for the feedback.
> >
> > > -----Original Message-----
> > > Subject: Re: [PATCH 3/6] rtc: da9063: Use dev_err_probe()
> > >
> > > On 01/12/2023 13:30:05+0000, Biju Das wrote:
> > > > Hi Geert,
> > > >
> > > > Thanks for the feedback.
> > > >
> > > > > -----Original Message-----
> > > > > From: Geert Uytterhoeven <geert@linux-m68k.org>
> > > > > Subject: Re: [PATCH 3/6] rtc: da9063: Use dev_err_probe()
> > > > >
> > > > > Hi Biju,
> > > > >
> > > > > On Fri, Dec 1, 2023 at 12:08 PM Biju Das
> > > > > <biju.das.jz@bp.renesas.com>
> > > > > wrote:
> > > > > > Replace dev_err()->dev_err_probe() to simpilfy probe().
> > > > > >
> > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > > >
> > > > > Thanks for your patch!
> > > > >
> > > > > > --- a/drivers/rtc/rtc-da9063.c
> > > > > > +++ b/drivers/rtc/rtc-da9063.c
> > > > > > @@ -488,8 +480,9 @@ static int da9063_rtc_probe(struct
> > > > > > platform_device
> > > > > *pdev)
> > > > > >
> > > > > > IRQF_TRIGGER_LOW |
> > > > > IRQF_ONESHOT,
> > > > > >                                                 "ALARM", rtc);
> > > > > >                 if (ret)
> > > > > > -                       dev_err(&pdev->dev, "Failed to request
> ALARM
> > > > > IRQ %d: %d\n",
> > > > > > -                               irq_alarm, ret);
> > > > > > +                       return dev_err_probe(&pdev->dev, ret,
> > > > > > +                                            "Failed to
> > > > > > + request ALARM
> > > > > IRQ %d\n",
> > > > > > +                                            irq_alarm);
> > > > >
> > > > > This changes behavior: before, this was not considered fatal.
> > > >
> > > > Agreed. Maybe a separate patch?
> > > >
> > > > if there is no irqhandler on platform with IRQ populated nothing
> > > > will work, RTC won't work as "rtc_update_irq " updated in irq
> handler.
> > > > I think it is a fatal condition.
> > >
> > > This is not true, an RTC can wake up a system without an IRQ.
> >
> > Agreed, I will restore the behaviour for this case.
> > It may not be fatal. But basic "hwclock -r" from userspace won't work
> > as " RTC_FEATURE_UPDATE_INTERRUPT" set and there is no irqhandler.
> >
> 
> RTC_FEATURE_ALARM is what you should clear and you have to fallback to the
> irq is not present case.


Ok,Will update Patch#3 with clearing "RTC_FEATURE_ALARM" as the fallback for the irqhandler error case

On patch#1, I will clear both RTC_FEATURE_ALARM" and "RTC_FEATURE_UPDATE_INTERRUPT",

as with just clearing "RTC_FEATURE_ALARM", I get below error.

root@smarc-rzg2ul:~# date -s "2023-08-06 19:30:00"
Sun Aug  6 19:30:00 UTC 2023
root@smarc-rzg2ul:~# hwclock -w
root@smarc-rzg2ul:~# hwclock -r
hwclock: select() to /dev/rtc0 to wait for clock tick timed out
root@smarc-rzg2ul:~#


Cheers,
Biju

> > > > >
> > > > > >
> > > > > >                 ret = dev_pm_set_wake_irq(&pdev->dev,
> irq_alarm);
> > > > > >                 if (ret)
> > > > >
> > > > > The rest LGTM, so with the above fixed/clarified:
> > > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > > >
> > > > > Gr{oetje,eeting}s,
> > > > >
> > > > >                         Geert
> > > > >
> > > > > --
> > > > > Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> > > > > geert@linux- m68k.org
> > > > >
> > > > > In personal conversations with technical people, I call myself a
> > > hacker.
> > > > > But when I'm talking to journalists I just say "programmer" or
> > > > > something like that.
> > > > >                                 -- Linus Torvalds
> > >
> > > --
> > > Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and
> > > Kernel engineering
> > >
> https://jpn01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbootlin%
> 2F&data=05%7C01%7Cbiju.das.jz%40bp.renesas.com%7C72e93f2d3b25447789c608dbf
> 285e823%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C638370429169364269%7C
> Unknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwi
> LCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=zGt9Zsk6AYZ3zwOTU6l0zmN3KF1rGqOTAe3XR
> hxPWaA%3D&reserved=0.
> > > com%2F&data=05%7C01%7Cbiju.das.jz%40bp.renesas.com%7Cb699f48656d34a9
> > > 23a640
> > > 8dbf28104af%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C63837040817
> > > 604431
> > > 5%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTi
> > > I6Ik1h
> > > aWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=E9tDi08sBRoh4tBccQB%2B8az%2
> > > BqQ4%2
> > > FtQOpFjdPgU8zQXc%3D&reserved=0
> 
> --
> Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and Kernel
> engineering
> https://jpn01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbootlin.
> com%2F&data=05%7C01%7Cbiju.das.jz%40bp.renesas.com%7C72e93f2d3b25447789c60
> 8dbf285e823%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C63837042916952053
> 1%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1h
> aWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=WULSktePlojGqVPbQJ%2BDelJnQEOUIh%
> 2BaSJm2Ra4OsRI%3D&reserved=0

Re: [PATCH 3/6] rtc: da9063: Use dev_err_probe()

[Alexandre Belloni]

On 01/12/2023 16:40:25+0000, Biju Das wrote:
> > RTC_FEATURE_ALARM is what you should clear and you have to fallback to the
> > irq is not present case.
> 
> 
> Ok,Will update Patch#3 with clearing "RTC_FEATURE_ALARM" as the fallback for the irqhandler error case
> 
> On patch#1, I will clear both RTC_FEATURE_ALARM" and "RTC_FEATURE_UPDATE_INTERRUPT",
> 
> as with just clearing "RTC_FEATURE_ALARM", I get below error.
> 
> root@smarc-rzg2ul:~# date -s "2023-08-06 19:30:00"
> Sun Aug  6 19:30:00 UTC 2023
> root@smarc-rzg2ul:~# hwclock -w
> root@smarc-rzg2ul:~# hwclock -r
> hwclock: select() to /dev/rtc0 to wait for clock tick timed out
> root@smarc-rzg2ul:~#
> 
>

I can't believe this is true because the rtc core code will take the
same path when RTC_FEATURE_ALARM or RTC_FEATURE_UPDATE_INTERRUPT is
cleared:

https://elixir.bootlin.com/linux/latest/source/drivers/rtc/interface.c#L574

RTC_FEATURE_UPDATE_INTERRUPT must be cleared only when RTC_FEATURE_ALARM
is set.


> Cheers,
> Biju
> 
> > > > > >
> > > > > > >
> > > > > > >                 ret = dev_pm_set_wake_irq(&pdev->dev,
> > irq_alarm);
> > > > > > >                 if (ret)
> > > > > >
> > > > > > The rest LGTM, so with the above fixed/clarified:
> > > > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > > > >
> > > > > > Gr{oetje,eeting}s,
> > > > > >
> > > > > >                         Geert
> > > > > >
> > > > > > --
> > > > > > Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> > > > > > geert@linux- m68k.org
> > > > > >
> > > > > > In personal conversations with technical people, I call myself a
> > > > hacker.
> > > > > > But when I'm talking to journalists I just say "programmer" or
> > > > > > something like that.
> > > > > >                                 -- Linus Torvalds
> > > >
> > > > --
> > > > Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and
> > > > Kernel engineering
> > > >
> > https://jpn01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbootlin%
> > 2F&data=05%7C01%7Cbiju.das.jz%40bp.renesas.com%7C72e93f2d3b25447789c608dbf
> > 285e823%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C638370429169364269%7C
> > Unknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwi
> > LCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=zGt9Zsk6AYZ3zwOTU6l0zmN3KF1rGqOTAe3XR
> > hxPWaA%3D&reserved=0.
> > > > com%2F&data=05%7C01%7Cbiju.das.jz%40bp.renesas.com%7Cb699f48656d34a9
> > > > 23a640
> > > > 8dbf28104af%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C63837040817
> > > > 604431
> > > > 5%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTi
> > > > I6Ik1h
> > > > aWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=E9tDi08sBRoh4tBccQB%2B8az%2
> > > > BqQ4%2
> > > > FtQOpFjdPgU8zQXc%3D&reserved=0
> > 
> > --
> > Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and Kernel
> > engineering
> > https://jpn01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbootlin.
> > com%2F&data=05%7C01%7Cbiju.das.jz%40bp.renesas.com%7C72e93f2d3b25447789c60
> > 8dbf285e823%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C63837042916952053
> > 1%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1h
> > aWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=WULSktePlojGqVPbQJ%2BDelJnQEOUIh%
> > 2BaSJm2Ra4OsRI%3D&reserved=0

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

Re: [PATCH 3/6] rtc: da9063: Use dev_err_probe()

[Alexandre Belloni]

On 20/12/2023 00:49:57+0100, Alexandre Belloni wrote:
> On 01/12/2023 16:40:25+0000, Biju Das wrote:
> > > RTC_FEATURE_ALARM is what you should clear and you have to fallback to the
> > > irq is not present case.
> > 
> > 
> > Ok,Will update Patch#3 with clearing "RTC_FEATURE_ALARM" as the fallback for the irqhandler error case
> > 
> > On patch#1, I will clear both RTC_FEATURE_ALARM" and "RTC_FEATURE_UPDATE_INTERRUPT",
> > 
> > as with just clearing "RTC_FEATURE_ALARM", I get below error.
> > 
> > root@smarc-rzg2ul:~# date -s "2023-08-06 19:30:00"
> > Sun Aug  6 19:30:00 UTC 2023
> > root@smarc-rzg2ul:~# hwclock -w
> > root@smarc-rzg2ul:~# hwclock -r
> > hwclock: select() to /dev/rtc0 to wait for clock tick timed out
> > root@smarc-rzg2ul:~#
> > 
> >
> 
> I can't believe this is true because the rtc core code will take the
> same path when RTC_FEATURE_ALARM or RTC_FEATURE_UPDATE_INTERRUPT is
> cleared:
> 
> https://elixir.bootlin.com/linux/latest/source/drivers/rtc/interface.c#L574
> 
> RTC_FEATURE_UPDATE_INTERRUPT must be cleared only when RTC_FEATURE_ALARM
> is set.

Are you sure you didn't break this test:

https://elixir.bootlin.com/linux/latest/source/drivers/rtc/rtc-da9063.c#L479

> 
> 
> > Cheers,
> > Biju
> > 
> > > > > > >
> > > > > > > >
> > > > > > > >                 ret = dev_pm_set_wake_irq(&pdev->dev,
> > > irq_alarm);
> > > > > > > >                 if (ret)
> > > > > > >
> > > > > > > The rest LGTM, so with the above fixed/clarified:
> > > > > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > > > > >
> > > > > > > Gr{oetje,eeting}s,
> > > > > > >
> > > > > > >                         Geert
> > > > > > >
> > > > > > > --
> > > > > > > Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> > > > > > > geert@linux- m68k.org
> > > > > > >
> > > > > > > In personal conversations with technical people, I call myself a
> > > > > hacker.
> > > > > > > But when I'm talking to journalists I just say "programmer" or
> > > > > > > something like that.
> > > > > > >                                 -- Linus Torvalds
> > > > >
> > > > > --
> > > > > Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and
> > > > > Kernel engineering
> > > > >
> > > https://jpn01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbootlin%
> > > 2F&data=05%7C01%7Cbiju.das.jz%40bp.renesas.com%7C72e93f2d3b25447789c608dbf
> > > 285e823%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C638370429169364269%7C
> > > Unknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwi
> > > LCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=zGt9Zsk6AYZ3zwOTU6l0zmN3KF1rGqOTAe3XR
> > > hxPWaA%3D&reserved=0.
> > > > > com%2F&data=05%7C01%7Cbiju.das.jz%40bp.renesas.com%7Cb699f48656d34a9
> > > > > 23a640
> > > > > 8dbf28104af%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C63837040817
> > > > > 604431
> > > > > 5%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTi
> > > > > I6Ik1h
> > > > > aWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=E9tDi08sBRoh4tBccQB%2B8az%2
> > > > > BqQ4%2
> > > > > FtQOpFjdPgU8zQXc%3D&reserved=0
> > > 
> > > --
> > > Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and Kernel
> > > engineering
> > > https://jpn01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbootlin.
> > > com%2F&data=05%7C01%7Cbiju.das.jz%40bp.renesas.com%7C72e93f2d3b25447789c60
> > > 8dbf285e823%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C63837042916952053
> > > 1%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1h
> > > aWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=WULSktePlojGqVPbQJ%2BDelJnQEOUIh%
> > > 2BaSJm2Ra4OsRI%3D&reserved=0
> 
> -- 
> Alexandre Belloni, co-owner and COO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

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

RE: [PATCH 3/6] rtc: da9063: Use dev_err_probe()

[Biju Das]

Hi Alexandre Belloni,

> -----Original Message-----
> From: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Sent: Tuesday, December 19, 2023 11:50 PM
> Subject: Re: [PATCH 3/6] rtc: da9063: Use dev_err_probe()
> 
> On 01/12/2023 16:40:25+0000, Biju Das wrote:
> > > RTC_FEATURE_ALARM is what you should clear and you have to fallback
> > > to the irq is not present case.
> >
> >
> > Ok,Will update Patch#3 with clearing "RTC_FEATURE_ALARM" as the
> > fallback for the irqhandler error case
> >
> > On patch#1, I will clear both RTC_FEATURE_ALARM" and
> > "RTC_FEATURE_UPDATE_INTERRUPT",
> >
> > as with just clearing "RTC_FEATURE_ALARM", I get below error.
> >
> > root@smarc-rzg2ul:~# date -s "2023-08-06 19:30:00"
> > Sun Aug  6 19:30:00 UTC 2023
> > root@smarc-rzg2ul:~# hwclock -w
> > root@smarc-rzg2ul:~# hwclock -r
> > hwclock: select() to /dev/rtc0 to wait for clock tick timed out
> > root@smarc-rzg2ul:~#
> >
> >
> 
> I can't believe this is true because the rtc core code will take the same
> path when RTC_FEATURE_ALARM or RTC_FEATURE_UPDATE_INTERRUPT is
> cleared:
> 
> 
> RTC_FEATURE_UPDATE_INTERRUPT must be cleared only when RTC_FEATURE_ALARM
> is set.

I rechecked this with latest next and it is working with clearing RTC_FEATURE_ALARM.

I will send v3 with this change.

root@smarc-rzg2ul:~# date -s "2023-08-06 19:30:00"
Sun Aug  6 19:30:00 UTC 2023
root@smarc-rzg2ul:~# hwclock -w
root@smarc-rzg2ul:~# hwclock -r
2023-08-06 19:30:11.664350+00:00
root@smarc-rzg2ul:~#

Cheers,
Biju