[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 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


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[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

[PATCH 4/6] dt-bindings: mfd: Convert da9062 to json-schema

[Biju Das]

Convert the da9062 PMIC device tree binding documentation to json-schema.

Update the example to match reality.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 .../devicetree/bindings/mfd/da9062.txt        | 124 ----------
 .../devicetree/bindings/mfd/dlg,da9062.yaml   | 220 ++++++++++++++++++
 2 files changed, 220 insertions(+), 124 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/mfd/da9062.txt
 create mode 100644 Documentation/devicetree/bindings/mfd/dlg,da9062.yaml

diff --git a/Documentation/devicetree/bindings/mfd/da9062.txt b/Documentation/devicetree/bindings/mfd/da9062.txt
deleted file mode 100644
index e4eedd3bd233..000000000000
--- a/Documentation/devicetree/bindings/mfd/da9062.txt
+++ /dev/null
@@ -1,124 +0,0 @@
-* Dialog DA9062 Power Management Integrated Circuit (PMIC)
-
-Product information for the DA9062 and DA9061 devices can be found here:
-- https://www.dialog-semiconductor.com/products/da9062
-- https://www.dialog-semiconductor.com/products/da9061
-
-The DA9062 PMIC consists of:
-
-Device                   Supply Names    Description
-------                   ------------    -----------
-da9062-regulator        :               : LDOs & BUCKs
-da9062-rtc              :               : Real-Time Clock
-da9062-onkey            :               : On Key
-da9062-watchdog         :               : Watchdog Timer
-da9062-thermal          :               : Thermal
-da9062-gpio             :               : GPIOs
-
-The DA9061 PMIC consists of:
-
-Device                   Supply Names    Description
-------                   ------------    -----------
-da9062-regulator        :               : LDOs & BUCKs
-da9062-onkey            :               : On Key
-da9062-watchdog         :               : Watchdog Timer
-da9062-thermal          :               : Thermal
-
-======
-
-Required properties:
-
-- compatible : Should be
-    "dlg,da9062" for DA9062
-    "dlg,da9061" for DA9061
-- reg : Specifies the I2C slave address (this defaults to 0x58 but it can be
-  modified to match the chip's OTP settings).
-
-Optional properties:
-
-- gpio-controller : Marks the device as a gpio controller.
-- #gpio-cells     : Should be two. The first cell is the pin number and the
-                    second cell is used to specify the gpio polarity.
-
-See Documentation/devicetree/bindings/gpio/gpio.txt for further information on
-GPIO bindings.
-
-- interrupts : IRQ line information.
-- interrupt-controller
-
-See Documentation/devicetree/bindings/interrupt-controller/interrupts.txt for
-further information on IRQ bindings.
-
-Sub-nodes:
-
-- regulators : This node defines the settings for the LDOs and BUCKs.
-  The DA9062 regulators are bound using their names listed below:
-
-    buck1    : BUCK_1
-    buck2    : BUCK_2
-    buck3    : BUCK_3
-    buck4    : BUCK_4
-    ldo1     : LDO_1
-    ldo2     : LDO_2
-    ldo3     : LDO_3
-    ldo4     : LDO_4
-
-  The DA9061 regulators are bound using their names listed below:
-
-    buck1    : BUCK_1
-    buck2    : BUCK_2
-    buck3    : BUCK_3
-    ldo1     : LDO_1
-    ldo2     : LDO_2
-    ldo3     : LDO_3
-    ldo4     : LDO_4
-
-  The component follows the standard regulator framework and the bindings
-  details of individual regulator device can be found in:
-  Documentation/devicetree/bindings/regulator/regulator.txt
-
-  regulator-initial-mode may be specified for buck regulators using mode values
-  from include/dt-bindings/regulator/dlg,da9063-regulator.h.
-
-- rtc : This node defines settings required for the Real-Time Clock associated
-  with the DA9062. There are currently no entries in this binding, however
-  compatible = "dlg,da9062-rtc" should be added if a node is created.
-
-- onkey : See ../input/da9062-onkey.txt
-
-- watchdog: See ../watchdog/da9062-wdt.txt
-
-- thermal : See ../thermal/da9062-thermal.txt
-
-Example:
-
-	pmic0: da9062@58 {
-		compatible = "dlg,da9062";
-		reg = <0x58>;
-		interrupt-parent = <&gpio6>;
-		interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
-		interrupt-controller;
-
-		rtc {
-			compatible = "dlg,da9062-rtc";
-		};
-
-		regulators {
-			DA9062_BUCK1: buck1 {
-				regulator-name = "BUCK1";
-				regulator-min-microvolt = <300000>;
-				regulator-max-microvolt = <1570000>;
-				regulator-min-microamp = <500000>;
-				regulator-max-microamp = <2000000>;
-				regulator-initial-mode = <DA9063_BUCK_MODE_SYNC>;
-				regulator-boot-on;
-			};
-			DA9062_LDO1: ldo1 {
-				regulator-name = "LDO_1";
-				regulator-min-microvolt = <900000>;
-				regulator-max-microvolt = <3600000>;
-				regulator-boot-on;
-			};
-		};
-	};
-
diff --git a/Documentation/devicetree/bindings/mfd/dlg,da9062.yaml b/Documentation/devicetree/bindings/mfd/dlg,da9062.yaml
new file mode 100644
index 000000000000..43ddf14a4a6d
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/dlg,da9062.yaml
@@ -0,0 +1,220 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/dlg,da9062.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Dialog DA9062 Power Management Integrated Circuit (PMIC)
+
+maintainers:
+  - Biju Das <biju.das.jz@bp.renesas.com>
+
+description: |
+  Product information for the DA9062 and DA9061 devices can be found here:
+  - https://www.dialog-semiconductor.com/products/da9062
+  - https://www.dialog-semiconductor.com/products/da9061
+
+  The DA9062 PMIC consists of:
+
+  Device                   Supply Names    Description
+  ------                   ------------    -----------
+  da9062-regulator        :               : LDOs & BUCKs
+  da9062-rtc              :               : Real-Time Clock
+  da9062-onkey            :               : On Key
+  da9062-watchdog         :               : Watchdog Timer
+  da9062-thermal          :               : Thermal
+  da9062-gpio             :               : GPIOs
+
+  The DA9061 PMIC consists of:
+
+  Device                   Supply Names    Description
+  ------                   ------------    -----------
+  da9062-regulator        :               : LDOs & BUCKs
+  da9062-onkey            :               : On Key
+  da9062-watchdog         :               : Watchdog Timer
+  da9062-thermal          :               : Thermal
+
+properties:
+  compatible:
+    enum:
+      - dlg,da9062
+      - dlg,da9061
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  interrupt-controller: true
+
+  "#interrupt-cells":
+    const: 2
+
+  gpio-controller: true
+
+  "#gpio-cells":
+    const: 2
+
+  watchdog:
+    type: object
+    $ref: /schemas/watchdog/watchdog.yaml#
+    unevaluatedProperties: false
+    properties:
+      compatible:
+        const: dlg,da9062-watchdog
+
+      dlg,use-sw-pm:
+        type: boolean
+        description:
+          Disable the watchdog during suspend.
+          Only use this option if you can't use the watchdog automatic suspend
+          function during a suspend (see register CONTROL_B).
+
+  rtc:
+    type: object
+    $ref: /schemas/rtc/rtc.yaml#
+    unevaluatedProperties: false
+    properties:
+      compatible:
+        const: dlg,da9062-rtc
+
+  thermal:
+    type: object
+    unevaluatedProperties: false
+    properties:
+      compatible:
+        const: dlg,da9062-thermal
+
+  gpio:
+    type: object
+    $ref: /schemas/gpio/gpio.yaml#
+    unevaluatedProperties: false
+    properties:
+      compatible:
+        const: dlg,da9062-gpio
+
+  onkey:
+    type: object
+    $ref: /schemas/input/input.yaml#
+    unevaluatedProperties: false
+    properties:
+      compatible:
+        const: dlg,da9062-onkey
+
+      dlg,disable-key-power:
+        type: boolean
+        description: |
+          Disable power-down using a long key-press.
+          If this entry does not exist then by default the key-press triggered
+          power down is enabled and the OnKey will support both KEY_POWER and
+          KEY_SLEEP.
+
+  regulators:
+    type: object
+    additionalProperties: false
+    patternProperties:
+      "^(ldo[1-9]|buck[1-4])$":
+        $ref: /schemas/regulator/regulator.yaml
+        unevaluatedProperties: false
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/regulator/dlg,da9063-regulator.h>
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      pmic@58 {
+        compatible = "dlg,da9062";
+        reg = <0x58>;
+        #interrupt-cells = <2>;
+        interrupt-parent = <&gpio1>;
+        interrupts = <2 IRQ_TYPE_LEVEL_LOW>;
+        interrupt-controller;
+
+        rtc {
+          compatible = "dlg,da9062-rtc";
+        };
+
+        onkey {
+          compatible = "dlg,da9062-onkey";
+        };
+
+        watchdog {
+          compatible = "dlg,da9062-watchdog";
+          dlg,use-sw-pm;
+        };
+
+        thermal {
+          compatible = "dlg,da9062-thermal";
+          status = "disabled";
+        };
+
+        gpio {
+          compatible = "dlg,da9062-gpio";
+          status = "disabled";
+        };
+
+        regulators {
+          buck1 {
+            regulator-name = "vdd_arm";
+            regulator-min-microvolt = <925000>;
+            regulator-max-microvolt = <1380000>;
+            regulator-initial-mode = <DA9063_BUCK_MODE_SYNC>;
+            regulator-always-on;
+          };
+          buck2 {
+            regulator-name = "vdd_soc";
+            regulator-min-microvolt = <1150000>;
+            regulator-max-microvolt = <1380000>;
+            regulator-initial-mode = <DA9063_BUCK_MODE_SYNC>;
+            regulator-always-on;
+          };
+          buck3 {
+            regulator-name = "vdd_ddr3";
+            regulator-min-microvolt = <1500000>;
+            regulator-max-microvolt = <1500000>;
+            regulator-initial-mode = <DA9063_BUCK_MODE_SYNC>;
+            regulator-always-on;
+          };
+          buck4 {
+            regulator-name = "vdd_eth";
+            regulator-min-microvolt = <1200000>;
+            regulator-max-microvolt = <1200000>;
+            regulator-initial-mode = <DA9063_BUCK_MODE_SYNC>;
+            regulator-always-on;
+          };
+          ldo1 {
+            regulator-name = "vdd_snvs";
+            regulator-min-microvolt = <3000000>;
+            regulator-max-microvolt = <3000000>;
+            regulator-always-on;
+          };
+          ldo2 {
+            regulator-name = "vdd_high";
+            regulator-min-microvolt = <3000000>;
+            regulator-max-microvolt = <3000000>;
+            regulator-always-on;
+          };
+          ldo3 {
+            regulator-name = "vdd_eth_io";
+            regulator-min-microvolt = <2500000>;
+            regulator-max-microvolt = <2500000>;
+          };
+          ldo4 {
+            regulator-name = "vdd_emmc";
+            regulator-min-microvolt = <1800000>;
+            regulator-max-microvolt = <1800000>;
+            regulator-always-on;
+          };
+        };
+      };
+    };
+...
-- 
2.25.1

Re: [PATCH 4/6] dt-bindings: mfd: Convert da9062 to json-schema

[Geert Uytterhoeven]

Hi Biju,

On Fri, Dec 1, 2023 at 12:09 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> Convert the da9062 PMIC device tree binding documentation to json-schema.
>
> Update the example to match reality.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Thanks for your patch!

> --- a/Documentation/devicetree/bindings/mfd/da9062.txt
> +++ /dev/null

This file is still referred to from other files:

    $ git grep Documentation/devicetree/bindings/mfd/da9062.txt
    Documentation/devicetree/bindings/input/da9062-onkey.txt:DA9062
and DA9061 chips see Documentation/devicetree/bindings/mfd/da9062.txt
    Documentation/devicetree/bindings/thermal/da9062-thermal.txt:DA9062
and DA9061 chips see Documentation/devicetree/bindings/mfd/da9062.txt

> -- onkey : See ../input/da9062-onkey.txt

Documentation/devicetree/bindings/input/da9062-onkey.txt still exists,
and covers more variants than your new dlg,da9062.yaml.

> -
> -- watchdog: See ../watchdog/da9062-wdt.txt

This was replaced by
Documentation/devicetree/bindings/watchdog/dlg,da9062-watchdog.yaml
a while ago, and covers more variants than your new dlg,da9062.yaml.

> -
> -- thermal : See ../thermal/da9062-thermal.txt

Documentation/devicetree/bindings/thermal/da9062-thermal.txt still exists,
and covers more variants than your new dlg,da9062.yaml.

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/dlg,da9062.yaml
> @@ -0,0 +1,220 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/dlg,da9062.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Dialog DA9062 Power Management Integrated Circuit (PMIC)
> +
> +maintainers:
> +  - Biju Das <biju.das.jz@bp.renesas.com>
> +
> +description: |
> +  Product information for the DA9062 and DA9061 devices can be found here:
> +  - https://www.dialog-semiconductor.com/products/da9062
> +  - https://www.dialog-semiconductor.com/products/da9061
> +
> +  The DA9062 PMIC consists of:
> +
> +  Device                   Supply Names    Description
> +  ------                   ------------    -----------
> +  da9062-regulator        :               : LDOs & BUCKs
> +  da9062-rtc              :               : Real-Time Clock
> +  da9062-onkey            :               : On Key
> +  da9062-watchdog         :               : Watchdog Timer
> +  da9062-thermal          :               : Thermal
> +  da9062-gpio             :               : GPIOs
> +
> +  The DA9061 PMIC consists of:
> +
> +  Device                   Supply Names    Description
> +  ------                   ------------    -----------
> +  da9062-regulator        :               : LDOs & BUCKs
> +  da9062-onkey            :               : On Key
> +  da9062-watchdog         :               : Watchdog Timer
> +  da9062-thermal          :               : Thermal

da9061 (x4)

> +
> +properties:

> +  watchdog:

Please sort subnodes alphabetically.

> +    type: object
> +    $ref: /schemas/watchdog/watchdog.yaml#
> +    unevaluatedProperties: false
> +    properties:
> +      compatible:
> +        const: dlg,da9062-watchdog

What about dlg,da9061-watchdog? Probably this should refer to
Documentation/devicetree/bindings/watchdog/dlg,da9062-watchdog.yaml
instead?

This applies to the other subnodes, too.

Perhaps this binding should be merged with dlg,da9063.yaml?
Or should it be split in dlg,da9061.yaml and dlg,da9062.yaml?

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 4/6] dt-bindings: mfd: Convert da9062 to json-schema

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3968 bytes --]

On Fri, Dec 01, 2023 at 02:33:29PM +0100, Geert Uytterhoeven wrote:
> Hi Biju,
> 
> On Fri, Dec 1, 2023 at 12:09 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > Convert the da9062 PMIC device tree binding documentation to json-schema.
> >
> > Update the example to match reality.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Thanks for your patch!
> 
> > --- a/Documentation/devicetree/bindings/mfd/da9062.txt
> > +++ /dev/null
> 
> This file is still referred to from other files:
> 
>     $ git grep Documentation/devicetree/bindings/mfd/da9062.txt
>     Documentation/devicetree/bindings/input/da9062-onkey.txt:DA9062
> and DA9061 chips see Documentation/devicetree/bindings/mfd/da9062.txt
>     Documentation/devicetree/bindings/thermal/da9062-thermal.txt:DA9062
> and DA9061 chips see Documentation/devicetree/bindings/mfd/da9062.txt
> 
> > -- onkey : See ../input/da9062-onkey.txt
> 
> Documentation/devicetree/bindings/input/da9062-onkey.txt still exists,
> and covers more variants than your new dlg,da9062.yaml.
> 
> > -
> > -- watchdog: See ../watchdog/da9062-wdt.txt
> 
> This was replaced by
> Documentation/devicetree/bindings/watchdog/dlg,da9062-watchdog.yaml
> a while ago, and covers more variants than your new dlg,da9062.yaml.
> 
> > -
> > -- thermal : See ../thermal/da9062-thermal.txt
> 
> Documentation/devicetree/bindings/thermal/da9062-thermal.txt still exists,
> and covers more variants than your new dlg,da9062.yaml.
> 
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/dlg,da9062.yaml
> > @@ -0,0 +1,220 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/mfd/dlg,da9062.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Dialog DA9062 Power Management Integrated Circuit (PMIC)
> > +
> > +maintainers:
> > +  - Biju Das <biju.das.jz@bp.renesas.com>
> > +
> > +description: |
> > +  Product information for the DA9062 and DA9061 devices can be found here:
> > +  - https://www.dialog-semiconductor.com/products/da9062
> > +  - https://www.dialog-semiconductor.com/products/da9061
> > +
> > +  The DA9062 PMIC consists of:
> > +

> > +  Device                   Supply Names    Description
> > +  ------                   ------------    -----------
> > +  da9062-regulator        :               : LDOs & BUCKs
> > +  da9062-rtc              :               : Real-Time Clock
> > +  da9062-onkey            :               : On Key
> > +  da9062-watchdog         :               : Watchdog Timer
> > +  da9062-thermal          :               : Thermal
> > +  da9062-gpio             :               : GPIOs
> > +
> > +  The DA9061 PMIC consists of:
> > +
> > +  Device                   Supply Names    Description
> > +  ------                   ------------    -----------
> > +  da9062-regulator        :               : LDOs & BUCKs
> > +  da9062-onkey            :               : On Key
> > +  da9062-watchdog         :               : Watchdog Timer
> > +  da9062-thermal          :               : Thermal
> 
> da9061 (x4)

Is retaining this even needed with a yaml binding that correctly
constrains the children?

> 
> > +
> > +properties:
> 
> > +  watchdog:
> 
> Please sort subnodes alphabetically.
> 
> > +    type: object
> > +    $ref: /schemas/watchdog/watchdog.yaml#
> > +    unevaluatedProperties: false
> > +    properties:
> > +      compatible:
> > +        const: dlg,da9062-watchdog
> 
> What about dlg,da9061-watchdog? Probably this should refer to
> Documentation/devicetree/bindings/watchdog/dlg,da9062-watchdog.yaml
> instead?
> 
> This applies to the other subnodes, too.
> 

> Perhaps this binding should be merged with dlg,da9063.yaml?
> Or should it be split in dlg,da9061.yaml and dlg,da9062.yaml?

The former sounds like a noble goal to me.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 4/6] dt-bindings: mfd: Convert da9062 to json-schema

[kernel test robot]

Hi Biju,

kernel test robot noticed the following build warnings:

[auto build test WARNING on abelloni/rtc-next]
[also build test WARNING on lee-mfd/for-mfd-next geert-renesas-devel/next arm64/for-next/core linus/master v6.7-rc4 next-20231205]
[cannot apply to lee-mfd/for-mfd-fixes]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Biju-Das/rtc-da9063-Use-device_get_match_data/20231201-231120
base:   https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git rtc-next
patch link:    https://lore.kernel.org/r/20231201110840.37408-5-biju.das.jz%40bp.renesas.com
patch subject: [PATCH 4/6] dt-bindings: mfd: Convert da9062 to json-schema
reproduce: (https://download.01.org/0day-ci/archive/20231206/202312061323.Zk81yCZU-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202312061323.Zk81yCZU-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> Warning: Documentation/devicetree/bindings/input/da9062-onkey.txt references a file that doesn't exist: Documentation/devicetree/bindings/mfd/da9062.txt
>> Warning: Documentation/devicetree/bindings/thermal/da9062-thermal.txt references a file that doesn't exist: Documentation/devicetree/bindings/mfd/da9062.txt

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

RE: [PATCH 4/6] dt-bindings: mfd: Convert da9062 to json-schema

[Biju Das]

Hi kernel test robot,

> -----Original Message-----
> From: kernel test robot <lkp@intel.com>
> Subject: Re: [PATCH 4/6] dt-bindings: mfd: Convert da9062 to json-schema
> 
> Hi Biju,
> 
> kernel test robot noticed the following build warnings:
> 
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new
> version of the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes:
> 
> All warnings (new ones prefixed by >>):
> 
> >> Warning: Documentation/devicetree/bindings/input/da9062-onkey.txt
> >> references a file that doesn't exist:
> >> Documentation/devicetree/bindings/mfd/da9062.txt
> >> Warning: Documentation/devicetree/bindings/thermal/da9062-thermal.txt
> >> references a file that doesn't exist:
> >> Documentation/devicetree/bindings/mfd/da9062.txt
> 

There is an updated patch series available[1] and Please retest and
provide feedback if any?

[1] https://lore.kernel.org/all/20231204172510.35041-1-biju.das.jz@bp.renesas.com/

Cheers,
Biju

Re: [PATCH 4/6] dt-bindings: mfd: Convert da9062 to json-schema

[Lee Jones]

On Wed, 06 Dec 2023, Biju Das wrote:

> Hi kernel test robot,
> 
> > -----Original Message-----
> > From: kernel test robot <lkp@intel.com>
> > Subject: Re: [PATCH 4/6] dt-bindings: mfd: Convert da9062 to json-schema
> > 
> > Hi Biju,
> > 
> > kernel test robot noticed the following build warnings:
> > 
> > 
> > If you fix the issue in a separate patch/commit (i.e. not just a new
> > version of the same patch/commit), kindly add following tags
> > | Reported-by: kernel test robot <lkp@intel.com>
> > | Closes:
> > 
> > All warnings (new ones prefixed by >>):
> > 
> > >> Warning: Documentation/devicetree/bindings/input/da9062-onkey.txt
> > >> references a file that doesn't exist:
> > >> Documentation/devicetree/bindings/mfd/da9062.txt
> > >> Warning: Documentation/devicetree/bindings/thermal/da9062-thermal.txt
> > >> references a file that doesn't exist:
> > >> Documentation/devicetree/bindings/mfd/da9062.txt
> > 
> 
> There is an updated patch series available[1] and Please retest and
> provide feedback if any?

Do you often find yourself chatting with inanimate/automated services? :)

-- 
Lee Jones [李琼斯]

RE: [PATCH 4/6] dt-bindings: mfd: Convert da9062 to json-schema

[Biju Das]

Hi Lee Jones,

> -----Original Message-----
> From: Lee Jones <lee@kernel.org>
> Sent: Wednesday, December 6, 2023 1:28 PM
> Subject: Re: [PATCH 4/6] dt-bindings: mfd: Convert da9062 to json-schema
> 
> On Wed, 06 Dec 2023, Biju Das wrote:
> 
> > Hi kernel test robot,
> >
> > > -----Original Message-----
> > > From: kernel test robot <lkp@intel.com>
> > > Subject: Re: [PATCH 4/6] dt-bindings: mfd: Convert da9062 to
> > > json-schema
> > >
> > > Hi Biju,
> > >
> > > kernel test robot noticed the following build warnings:
> > >
> > >
> > > If you fix the issue in a separate patch/commit (i.e. not just a new
> > > version of the same patch/commit), kindly add following tags
> > > | Reported-by: kernel test robot <lkp@intel.com>
> > > | Closes:
> > >
> > > All warnings (new ones prefixed by >>):
> > >
> > > >> Warning: Documentation/devicetree/bindings/input/da9062-onkey.txt
> > > >> references a file that doesn't exist:
> > > >> Documentation/devicetree/bindings/mfd/da9062.txt
> > > >> Warning:
> > > >> Documentation/devicetree/bindings/thermal/da9062-thermal.txt
> > > >> references a file that doesn't exist:
> > > >> Documentation/devicetree/bindings/mfd/da9062.txt
> > >
> >
> > There is an updated patch series available[1] and Please retest and
> > provide feedback if any?
> 
> Do you often find yourself chatting with inanimate/automated services? :)

FYI, based on the previous experience, there are some people used to respond
even though it is addressed to kernel test robot <lkp@intel.com>.

I don't know about the current situation.

Cheers,
Biju

Re: [PATCH 4/6] dt-bindings: mfd: Convert da9062 to json-schema

[Liu, Yujie]

Hi Biju, Hi Lee,

On Wed, 2023-12-06 at 15:19 +0000, Biju Das wrote:
> Hi Lee Jones,
> 
> > -----Original Message-----
> > From: Lee Jones <lee@kernel.org>
> > Sent: Wednesday, December 6, 2023 1:28 PM
> > Subject: Re: [PATCH 4/6] dt-bindings: mfd: Convert da9062 to json-
> > schema
> > 
> > On Wed, 06 Dec 2023, Biju Das wrote:
> > 
> > > Hi kernel test robot,
> > > 
> > > > -----Original Message-----
> > > > From: kernel test robot <lkp@intel.com>
> > > > Subject: Re: [PATCH 4/6] dt-bindings: mfd: Convert da9062 to
> > > > json-schema
> > > > 
> > > > Hi Biju,
> > > > 
> > > > kernel test robot noticed the following build warnings:
> > > > 
> > > > 
> > > > If you fix the issue in a separate patch/commit (i.e. not just
> > > > a new
> > > > version of the same patch/commit), kindly add following tags
> > > > > Reported-by: kernel test robot <lkp@intel.com>
> > > > > Closes:
> > > > 
> > > > All warnings (new ones prefixed by >>):
> > > > 
> > > > > > Warning: Documentation/devicetree/bindings/input/da9062-
> > > > > > onkey.txt
> > > > > > references a file that doesn't exist:
> > > > > > Documentation/devicetree/bindings/mfd/da9062.txt
> > > > > > Warning:
> > > > > > Documentation/devicetree/bindings/thermal/da9062-
> > > > > > thermal.txt
> > > > > > references a file that doesn't exist:
> > > > > > Documentation/devicetree/bindings/mfd/da9062.txt
> > > > 
> > > 
> > > There is an updated patch series available[1] and Please retest
> > > and
> > > provide feedback if any?
> > 
> > Do you often find yourself chatting with inanimate/automated
> > services? :)
> 
> FYI, based on the previous experience, there are some people used to
> respond
> even though it is addressed to kernel test robot <lkp@intel.com>.
> 
> I don't know about the current situation.

There are indeed humans behind the robot. Currently we are:

Philip Li <philip.li@intel.com>
Oliver Sang <oliver.sang@intel.com>
Yujie Liu <yujie.liu@intel.com>

We will respond if developers need help or have feedback on the robot,
but sometimes we may not be able to respond in time due to bandwidth
overload. Sorry for any delayed reply.

As for Biju's request of testing the updated patch series, the robot
cannot support auto-testing for it yet. Sorry for this. We suggest
first trying the reproducer in the original report to see if the issue
can be reproduced, then applying the updated patch series to check if
the reported issue goes away. Please kindly contact us if any problems
using the reproducer.

Best Regards,
Yujie

[PATCH 5/6] arm64: dts: renesas: rzg2ul-smarc: Enable PMIC and built-in RTC

[Biju Das]

Enable PMIC DA9062 and the built-in RTC on the RZ/{G2UL,Five} SMARC
EVK.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 arch/arm64/boot/dts/renesas/rzg2ul-smarc.dtsi | 29 +++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/rzg2ul-smarc.dtsi b/arch/arm64/boot/dts/renesas/rzg2ul-smarc.dtsi
index de590996e10a..7ef33087e6f9 100644
--- a/arch/arm64/boot/dts/renesas/rzg2ul-smarc.dtsi
+++ b/arch/arm64/boot/dts/renesas/rzg2ul-smarc.dtsi
@@ -23,6 +23,35 @@ &cpu_dai {
 &i2c0 {
 	clock-frequency = <400000>;
 
+	da9062: pmic@58 {
+		compatible = "dlg,da9062";
+		reg = <0x58>;
+
+		da9062_rtc: rtc {
+			compatible = "dlg,da9062-rtc";
+		};
+
+		da9062_onkey: onkey {
+			compatible = "dlg,da9062-onkey";
+			status = "disabled";
+		};
+
+		watchdog {
+			compatible = "dlg,da9062-watchdog";
+			status = "disabled";
+		};
+
+		thermal {
+			compatible = "dlg,da9062-thermal";
+			status = "disabled";
+		};
+
+		gpio {
+			compatible = "dlg,da9062-gpio";
+			status = "disabled";
+		};
+	};
+
 	versa3: clock-generator@68 {
 		compatible = "renesas,5p35023";
 		reg = <0x68>;
-- 
2.25.1

Re: [PATCH 5/6] arm64: dts: renesas: rzg2ul-smarc: Enable PMIC and built-in RTC

[Geert Uytterhoeven]

Hi Biju,

On Fri, Dec 1, 2023 at 12:09 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> Enable PMIC DA9062 and the built-in RTC on the RZ/{G2UL,Five} SMARC
> EVK.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Thanks for your patch!

> --- a/arch/arm64/boot/dts/renesas/rzg2ul-smarc.dtsi
> +++ b/arch/arm64/boot/dts/renesas/rzg2ul-smarc.dtsi
> @@ -23,6 +23,35 @@ &cpu_dai {
>  &i2c0 {
>         clock-frequency = <400000>;
>
> +       da9062: pmic@58 {
> +               compatible = "dlg,da9062";
> +               reg = <0x58>;
> +
> +               da9062_rtc: rtc {
> +                       compatible = "dlg,da9062-rtc";
> +               };
> +
> +               da9062_onkey: onkey {
> +                       compatible = "dlg,da9062-onkey";
> +                       status = "disabled";

Why is this disabled? This is connected to the power button.

> +               };
> +
> +               watchdog {
> +                       compatible = "dlg,da9062-watchdog";
> +                       status = "disabled";
> +               };
> +
> +               thermal {
> +                       compatible = "dlg,da9062-thermal";
> +                       status = "disabled";
> +               };
> +
> +               gpio {
> +                       compatible = "dlg,da9062-gpio";
> +                       status = "disabled";
> +               };

Why are these three disabled?
If they are truly unused, you can just drop the nodes instead.

> +       };
> +
>         versa3: clock-generator@68 {
>                 compatible = "renesas,5p35023";
>                 reg = <0x68>;

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 5/6] arm64: dts: renesas: rzg2ul-smarc: Enable PMIC and built-in RTC

[Biju Das]

Hi Geert,

Thanks for the feedback.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: Friday, December 1, 2023 1:40 PM
> Subject: Re: [PATCH 5/6] arm64: dts: renesas: rzg2ul-smarc: Enable PMIC
> and built-in RTC
> 
> Hi Biju,
> 
> On Fri, Dec 1, 2023 at 12:09 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > Enable PMIC DA9062 and the built-in RTC on the RZ/{G2UL,Five} SMARC
> > EVK.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Thanks for your patch!
> 
> > --- a/arch/arm64/boot/dts/renesas/rzg2ul-smarc.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/rzg2ul-smarc.dtsi
> > @@ -23,6 +23,35 @@ &cpu_dai {
> >  &i2c0 {
> >         clock-frequency = <400000>;
> >
> > +       da9062: pmic@58 {
> > +               compatible = "dlg,da9062";
> > +               reg = <0x58>;
> > +
> > +               da9062_rtc: rtc {
> > +                       compatible = "dlg,da9062-rtc";
> > +               };
> > +
> > +               da9062_onkey: onkey {
> > +                       compatible = "dlg,da9062-onkey";
> > +                       status = "disabled";
> 
> Why is this disabled? This is connected to the power button.

I haven't tested this driver. OK I will test and enable it.

> 
> > +               };
> > +
> > +               watchdog {
> > +                       compatible = "dlg,da9062-watchdog";
> > +                       status = "disabled";
> > +               };
> > +
> > +               thermal {
> > +                       compatible = "dlg,da9062-thermal";
> > +                       status = "disabled";
> > +               };
> > +
> > +               gpio {
> > +                       compatible = "dlg,da9062-gpio";
> > +                       status = "disabled";
> > +               };
> 
> Why are these three disabled?
> If they are truly unused, you can just drop the nodes instead.

If I drop these, I get failed messages(eg: da9062-gpio: Failed to locate of_node [id: -1])
for these nodes.

Cheers,
Biju

> 
> > +       };
> > +
> >         versa3: clock-generator@68 {
> >                 compatible = "renesas,5p35023";
> >                 reg = <0x68>;
> 
> 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 5/6] arm64: dts: renesas: rzg2ul-smarc: Enable PMIC and built-in RTC

[Geert Uytterhoeven]

Hi Biju,

On Fri, Dec 1, 2023 at 2:48 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > From: Geert Uytterhoeven <geert@linux-m68k.org>
> > On Fri, Dec 1, 2023 at 12:09 PM Biju Das <biju.das.jz@bp.renesas.com>
> > wrote:
> > > Enable PMIC DA9062 and the built-in RTC on the RZ/{G2UL,Five} SMARC
> > > EVK.
> > >
> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> >
> > Thanks for your patch!
> >
> > > --- a/arch/arm64/boot/dts/renesas/rzg2ul-smarc.dtsi
> > > +++ b/arch/arm64/boot/dts/renesas/rzg2ul-smarc.dtsi
> > > @@ -23,6 +23,35 @@ &cpu_dai {
> > >  &i2c0 {
> > >         clock-frequency = <400000>;
> > >
> > > +       da9062: pmic@58 {
> > > +               compatible = "dlg,da9062";
> > > +               reg = <0x58>;
> > > +
> > > +               da9062_rtc: rtc {
> > > +                       compatible = "dlg,da9062-rtc";
> > > +               };
> > > +
> > > +               da9062_onkey: onkey {
> > > +                       compatible = "dlg,da9062-onkey";
> > > +                       status = "disabled";
> >
> > Why is this disabled? This is connected to the power button.
>
> I haven't tested this driver. OK I will test and enable it.
>
> >
> > > +               };
> > > +
> > > +               watchdog {
> > > +                       compatible = "dlg,da9062-watchdog";
> > > +                       status = "disabled";
> > > +               };
> > > +
> > > +               thermal {
> > > +                       compatible = "dlg,da9062-thermal";
> > > +                       status = "disabled";
> > > +               };
> > > +
> > > +               gpio {
> > > +                       compatible = "dlg,da9062-gpio";
> > > +                       status = "disabled";
> > > +               };
> >
> > Why are these three disabled?
> > If they are truly unused, you can just drop the nodes instead.
>
> If I drop these, I get failed messages(eg: da9062-gpio: Failed to locate of_node [id: -1])
> for these nodes.

IC... They are indeed mandatory, but ignored when disabled.
Hence the subnodes should be made required in the DT bindings.

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 6/6] arm64: defconfig: Enable Renesas DA9062 defconfig

[Biju Das]

Enable the config for the Renesas DA9062 PMIC and RTC, as it is
populated on RZ/G2UL SMARC EVK.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 arch/arm64/configs/defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 5ad2b841aafc..b68b122d1b1e 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -714,6 +714,7 @@ CONFIG_MFD_ALTERA_SYSMGR=y
 CONFIG_MFD_BD9571MWV=y
 CONFIG_MFD_AXP20X_I2C=y
 CONFIG_MFD_AXP20X_RSB=y
+CONFIG_MFD_DA9062=y
 CONFIG_MFD_EXYNOS_LPASS=m
 CONFIG_MFD_HI6421_PMIC=y
 CONFIG_MFD_HI655X_PMIC=y
@@ -1130,6 +1131,7 @@ CONFIG_RTC_DRV_RV8803=m
 CONFIG_RTC_DRV_S5M=y
 CONFIG_RTC_DRV_DS3232=y
 CONFIG_RTC_DRV_PCF2127=m
+CONFIG_RTC_DRV_DA9063=m
 CONFIG_RTC_DRV_EFI=y
 CONFIG_RTC_DRV_CROS_EC=y
 CONFIG_RTC_DRV_FSL_FTM_ALARM=m
-- 
2.25.1

[PATCH 6/6] arm64: defconfig: Enable Renesas DA9062 defconfig

[Biju Das]

Enable the config for the Renesas DA9062 PMIC and RTC, as it is
populated on RZ/G2UL SMARC EVK.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 arch/arm64/configs/defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 5ad2b841aafc..b68b122d1b1e 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -714,6 +714,7 @@ CONFIG_MFD_ALTERA_SYSMGR=y
 CONFIG_MFD_BD9571MWV=y
 CONFIG_MFD_AXP20X_I2C=y
 CONFIG_MFD_AXP20X_RSB=y
+CONFIG_MFD_DA9062=y
 CONFIG_MFD_EXYNOS_LPASS=m
 CONFIG_MFD_HI6421_PMIC=y
 CONFIG_MFD_HI655X_PMIC=y
@@ -1130,6 +1131,7 @@ CONFIG_RTC_DRV_RV8803=m
 CONFIG_RTC_DRV_S5M=y
 CONFIG_RTC_DRV_DS3232=y
 CONFIG_RTC_DRV_PCF2127=m
+CONFIG_RTC_DRV_DA9063=m
 CONFIG_RTC_DRV_EFI=y
 CONFIG_RTC_DRV_CROS_EC=y
 CONFIG_RTC_DRV_FSL_FTM_ALARM=m
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 6/6] arm64: defconfig: Enable Renesas DA9062 defconfig

[Geert Uytterhoeven]

Hi Biju,

On Fri, Dec 1, 2023 at 12:09 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> Enable the config for the Renesas DA9062 PMIC and RTC, as it is
> populated on RZ/G2UL SMARC EVK.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Thanks for your patch!

LGTM, but I would have expected (at least)

    CONFIG_INPUT_DA9063_ONKEY=m

too, cfr. my comments on patch 5/6.

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 6/6] arm64: defconfig: Enable Renesas DA9062 defconfig

[Geert Uytterhoeven]

Hi Biju,

On Fri, Dec 1, 2023 at 12:09 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> Enable the config for the Renesas DA9062 PMIC and RTC, as it is
> populated on RZ/G2UL SMARC EVK.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Thanks for your patch!

LGTM, but I would have expected (at least)

    CONFIG_INPUT_DA9063_ONKEY=m

too, cfr. my comments on patch 5/6.

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 6/6] arm64: defconfig: Enable Renesas DA9062 defconfig

[Biju Das]

Hi Geert,

Thanks for the feedback.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: Friday, December 1, 2023 1:41 PM
> Subject: Re: [PATCH 6/6] arm64: defconfig: Enable Renesas DA9062 defconfig
> 
> Hi Biju,
> 
> On Fri, Dec 1, 2023 at 12:09 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > Enable the config for the Renesas DA9062 PMIC and RTC, as it is
> > populated on RZ/G2UL SMARC EVK.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Thanks for your patch!
> 
> LGTM, but I would have expected (at least)
> 
>     CONFIG_INPUT_DA9063_ONKEY=m
> 
> too, cfr. my comments on patch 5/6.

I have tested this, but I get "da9063-onkey da9062-onkey: error -ENXIO: IRQ ONKEY not found"
So, I won't be able to test onkey events. So plaaning to disable this node.

Cheers,
Biju

> 
> 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 6/6] arm64: defconfig: Enable Renesas DA9062 defconfig

[Biju Das]

Hi Geert,

Thanks for the feedback.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: Friday, December 1, 2023 1:41 PM
> Subject: Re: [PATCH 6/6] arm64: defconfig: Enable Renesas DA9062 defconfig
> 
> Hi Biju,
> 
> On Fri, Dec 1, 2023 at 12:09 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > Enable the config for the Renesas DA9062 PMIC and RTC, as it is
> > populated on RZ/G2UL SMARC EVK.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Thanks for your patch!
> 
> LGTM, but I would have expected (at least)
> 
>     CONFIG_INPUT_DA9063_ONKEY=m
> 
> too, cfr. my comments on patch 5/6.

I have tested this, but I get "da9063-onkey da9062-onkey: error -ENXIO: IRQ ONKEY not found"
So, I won't be able to test onkey events. So plaaning to disable this node.

Cheers,
Biju

> 
> 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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 6/6] arm64: defconfig: Enable Renesas DA9062 defconfig

[Geert Uytterhoeven]

Hi Biju,

On Fri, Dec 1, 2023 at 4:02 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > From: Geert Uytterhoeven <geert@linux-m68k.org>
> > On Fri, Dec 1, 2023 at 12:09 PM Biju Das <biju.das.jz@bp.renesas.com>
> > wrote:
> > > Enable the config for the Renesas DA9062 PMIC and RTC, as it is
> > > populated on RZ/G2UL SMARC EVK.
> > >
> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> >
> > Thanks for your patch!
> >
> > LGTM, but I would have expected (at least)
> >
> >     CONFIG_INPUT_DA9063_ONKEY=m
> >
> > too, cfr. my comments on patch 5/6.
>
> I have tested this, but I get "da9063-onkey da9062-onkey: error -ENXIO: IRQ ONKEY not found"
> So, I won't be able to test onkey events. So plaaning to disable this node.

Indeed, currently drivers/input/misc/da9063_onkey.c requires an
interrupt to function.  I wonder if it can be made to work using a
timer and pure polling...

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 6/6] arm64: defconfig: Enable Renesas DA9062 defconfig

[Geert Uytterhoeven]

Hi Biju,

On Fri, Dec 1, 2023 at 4:02 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > From: Geert Uytterhoeven <geert@linux-m68k.org>
> > On Fri, Dec 1, 2023 at 12:09 PM Biju Das <biju.das.jz@bp.renesas.com>
> > wrote:
> > > Enable the config for the Renesas DA9062 PMIC and RTC, as it is
> > > populated on RZ/G2UL SMARC EVK.
> > >
> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> >
> > Thanks for your patch!
> >
> > LGTM, but I would have expected (at least)
> >
> >     CONFIG_INPUT_DA9063_ONKEY=m
> >
> > too, cfr. my comments on patch 5/6.
>
> I have tested this, but I get "da9063-onkey da9062-onkey: error -ENXIO: IRQ ONKEY not found"
> So, I won't be able to test onkey events. So plaaning to disable this node.

Indeed, currently drivers/input/misc/da9063_onkey.c requires an
interrupt to function.  I wonder if it can be made to work using a
timer and pure polling...

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH 6/6] arm64: defconfig: Enable Renesas DA9062 defconfig

[Biju Das]

Hi Geert Uytterhoeven,

Thanks for the feedback.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: Friday, December 1, 2023 3:42 PM
> Subject: Re: [PATCH 6/6] arm64: defconfig: Enable Renesas DA9062 defconfig
> 
> Hi Biju,
> 
> On Fri, Dec 1, 2023 at 4:02 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > > From: Geert Uytterhoeven <geert@linux-m68k.org> On Fri, Dec 1, 2023
> > > at 12:09 PM Biju Das <biju.das.jz@bp.renesas.com>
> > > wrote:
> > > > Enable the config for the Renesas DA9062 PMIC and RTC, as it is
> > > > populated on RZ/G2UL SMARC EVK.
> > > >
> > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > >
> > > Thanks for your patch!
> > >
> > > LGTM, but I would have expected (at least)
> > >
> > >     CONFIG_INPUT_DA9063_ONKEY=m
> > >
> > > too, cfr. my comments on patch 5/6.
> >
> > I have tested this, but I get "da9063-onkey da9062-onkey: error -ENXIO:
> IRQ ONKEY not found"
> > So, I won't be able to test onkey events. So plaaning to disable this
> node.
> 
> Indeed, currently drivers/input/misc/da9063_onkey.c requires an interrupt
> to function.  I wonder if it can be made to work using a timer and pure
> polling...

Yes, that is possible by either adding a debugfs to enable polling for testing.

Or 

a device tree property for adding poll delay, so that one can have polling support to test onkey driver.

Basically, on a production system we should avoid polling as it unnecessarily wakeup the system.

Please share your thoughts on this.

Cheers,
Biju


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH 6/6] arm64: defconfig: Enable Renesas DA9062 defconfig

[Biju Das]

Hi Geert Uytterhoeven,

Thanks for the feedback.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: Friday, December 1, 2023 3:42 PM
> Subject: Re: [PATCH 6/6] arm64: defconfig: Enable Renesas DA9062 defconfig
> 
> Hi Biju,
> 
> On Fri, Dec 1, 2023 at 4:02 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > > From: Geert Uytterhoeven <geert@linux-m68k.org> On Fri, Dec 1, 2023
> > > at 12:09 PM Biju Das <biju.das.jz@bp.renesas.com>
> > > wrote:
> > > > Enable the config for the Renesas DA9062 PMIC and RTC, as it is
> > > > populated on RZ/G2UL SMARC EVK.
> > > >
> > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > >
> > > Thanks for your patch!
> > >
> > > LGTM, but I would have expected (at least)
> > >
> > >     CONFIG_INPUT_DA9063_ONKEY=m
> > >
> > > too, cfr. my comments on patch 5/6.
> >
> > I have tested this, but I get "da9063-onkey da9062-onkey: error -ENXIO:
> IRQ ONKEY not found"
> > So, I won't be able to test onkey events. So plaaning to disable this
> node.
> 
> Indeed, currently drivers/input/misc/da9063_onkey.c requires an interrupt
> to function.  I wonder if it can be made to work using a timer and pure
> polling...

Yes, that is possible by either adding a debugfs to enable polling for testing.

Or 

a device tree property for adding poll delay, so that one can have polling support to test onkey driver.

Basically, on a production system we should avoid polling as it unnecessarily wakeup the system.

Please share your thoughts on this.

Cheers,
Biju

Re: [PATCH 6/6] arm64: defconfig: Enable Renesas DA9062 defconfig

[Geert Uytterhoeven]

Hi Biju,

On Sun, Dec 10, 2023 at 10:50 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > -----Original Message-----
> > From: Geert Uytterhoeven <geert@linux-m68k.org>
> > Sent: Friday, December 1, 2023 3:42 PM
> > Subject: Re: [PATCH 6/6] arm64: defconfig: Enable Renesas DA9062 defconfig
> >
> > On Fri, Dec 1, 2023 at 4:02 PM Biju Das <biju.das.jz@bp.renesas.com>
> > wrote:
> > > > From: Geert Uytterhoeven <geert@linux-m68k.org> On Fri, Dec 1, 2023
> > > > at 12:09 PM Biju Das <biju.das.jz@bp.renesas.com>
> > > > wrote:
> > > > > Enable the config for the Renesas DA9062 PMIC and RTC, as it is
> > > > > populated on RZ/G2UL SMARC EVK.
> > > > >
> > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > >
> > > > Thanks for your patch!
> > > >
> > > > LGTM, but I would have expected (at least)
> > > >
> > > >     CONFIG_INPUT_DA9063_ONKEY=m
> > > >
> > > > too, cfr. my comments on patch 5/6.
> > >
> > > I have tested this, but I get "da9063-onkey da9062-onkey: error -ENXIO:
> > IRQ ONKEY not found"
> > > So, I won't be able to test onkey events. So plaaning to disable this
> > node.
> >
> > Indeed, currently drivers/input/misc/da9063_onkey.c requires an interrupt
> > to function.  I wonder if it can be made to work using a timer and pure
> > polling...
>
> Yes, that is possible by either adding a debugfs to enable polling for testing.
>
> Or
>
> a device tree property for adding poll delay, so that one can have polling support to test onkey driver.

I would go for the latter, cfr. poll-interval in
Documentation/devicetree/bindings/input/gpio-keys.yaml.

> Basically, on a production system we should avoid polling as it unnecessarily wakeup the system.

Wouldn't polling (by Linux) only happen when the system is already
awake?  I think the DA9063 itself handles the onkey when the system
is asleep.

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 6/6] arm64: defconfig: Enable Renesas DA9062 defconfig

[Geert Uytterhoeven]

Hi Biju,

On Sun, Dec 10, 2023 at 10:50 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > -----Original Message-----
> > From: Geert Uytterhoeven <geert@linux-m68k.org>
> > Sent: Friday, December 1, 2023 3:42 PM
> > Subject: Re: [PATCH 6/6] arm64: defconfig: Enable Renesas DA9062 defconfig
> >
> > On Fri, Dec 1, 2023 at 4:02 PM Biju Das <biju.das.jz@bp.renesas.com>
> > wrote:
> > > > From: Geert Uytterhoeven <geert@linux-m68k.org> On Fri, Dec 1, 2023
> > > > at 12:09 PM Biju Das <biju.das.jz@bp.renesas.com>
> > > > wrote:
> > > > > Enable the config for the Renesas DA9062 PMIC and RTC, as it is
> > > > > populated on RZ/G2UL SMARC EVK.
> > > > >
> > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > >
> > > > Thanks for your patch!
> > > >
> > > > LGTM, but I would have expected (at least)
> > > >
> > > >     CONFIG_INPUT_DA9063_ONKEY=m
> > > >
> > > > too, cfr. my comments on patch 5/6.
> > >
> > > I have tested this, but I get "da9063-onkey da9062-onkey: error -ENXIO:
> > IRQ ONKEY not found"
> > > So, I won't be able to test onkey events. So plaaning to disable this
> > node.
> >
> > Indeed, currently drivers/input/misc/da9063_onkey.c requires an interrupt
> > to function.  I wonder if it can be made to work using a timer and pure
> > polling...
>
> Yes, that is possible by either adding a debugfs to enable polling for testing.
>
> Or
>
> a device tree property for adding poll delay, so that one can have polling support to test onkey driver.

I would go for the latter, cfr. poll-interval in
Documentation/devicetree/bindings/input/gpio-keys.yaml.

> Basically, on a production system we should avoid polling as it unnecessarily wakeup the system.

Wouldn't polling (by Linux) only happen when the system is already
awake?  I think the DA9063 itself handles the onkey when the system
is asleep.

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH 6/6] arm64: defconfig: Enable Renesas DA9062 defconfig

[Biju Das]

Hi Geert,

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: Monday, December 11, 2023 8:20 AM
> Subject: Re: [PATCH 6/6] arm64: defconfig: Enable Renesas DA9062 defconfig
> 
> Hi Biju,
> 
> On Sun, Dec 10, 2023 at 10:50 AM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > > -----Original Message-----
> > > From: Geert Uytterhoeven <geert@linux-m68k.org>
> > > Sent: Friday, December 1, 2023 3:42 PM
> > > Subject: Re: [PATCH 6/6] arm64: defconfig: Enable Renesas DA9062
> > > defconfig
> > >
> > > On Fri, Dec 1, 2023 at 4:02 PM Biju Das <biju.das.jz@bp.renesas.com>
> > > wrote:
> > > > > From: Geert Uytterhoeven <geert@linux-m68k.org> On Fri, Dec 1,
> > > > > 2023 at 12:09 PM Biju Das <biju.das.jz@bp.renesas.com>
> > > > > wrote:
> > > > > > Enable the config for the Renesas DA9062 PMIC and RTC, as it
> > > > > > is populated on RZ/G2UL SMARC EVK.
> > > > > >
> > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > > >
> > > > > Thanks for your patch!
> > > > >
> > > > > LGTM, but I would have expected (at least)
> > > > >
> > > > >     CONFIG_INPUT_DA9063_ONKEY=m
> > > > >
> > > > > too, cfr. my comments on patch 5/6.
> > > >
> > > > I have tested this, but I get "da9063-onkey da9062-onkey: error -
> ENXIO:
> > > IRQ ONKEY not found"
> > > > So, I won't be able to test onkey events. So plaaning to disable
> > > > this
> > > node.
> > >
> > > Indeed, currently drivers/input/misc/da9063_onkey.c requires an
> > > interrupt to function.  I wonder if it can be made to work using a
> > > timer and pure polling...
> >
> > Yes, that is possible by either adding a debugfs to enable polling for
> testing.
> >
> > Or
> >
> > a device tree property for adding poll delay, so that one can have
> polling support to test onkey driver.
> 
> I would go for the latter, cfr. poll-interval in
> Documentation/devicetree/bindings/input/gpio-keys.yaml.
> 
> > Basically, on a production system we should avoid polling as it
> unnecessarily wakeup the system.
> 
> Wouldn't polling (by Linux) only happen when the system is already awake?

IIUC, That is an overhead. If there is no load, system goes to suspend mode and
the timer for the polling (SCAN keys) will wakeup the system and same 
process repeats. If we have a PMIC IRQ, this won't happen.

> I think the DA9063 itself handles the onkey when the system is asleep.

I will ask HW person to wire IRQ on PMIC, so that I can check the behaviour.

Cheers,
Biju

RE: [PATCH 6/6] arm64: defconfig: Enable Renesas DA9062 defconfig

[Biju Das]

Hi Geert,

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: Monday, December 11, 2023 8:20 AM
> Subject: Re: [PATCH 6/6] arm64: defconfig: Enable Renesas DA9062 defconfig
> 
> Hi Biju,
> 
> On Sun, Dec 10, 2023 at 10:50 AM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > > -----Original Message-----
> > > From: Geert Uytterhoeven <geert@linux-m68k.org>
> > > Sent: Friday, December 1, 2023 3:42 PM
> > > Subject: Re: [PATCH 6/6] arm64: defconfig: Enable Renesas DA9062
> > > defconfig
> > >
> > > On Fri, Dec 1, 2023 at 4:02 PM Biju Das <biju.das.jz@bp.renesas.com>
> > > wrote:
> > > > > From: Geert Uytterhoeven <geert@linux-m68k.org> On Fri, Dec 1,
> > > > > 2023 at 12:09 PM Biju Das <biju.das.jz@bp.renesas.com>
> > > > > wrote:
> > > > > > Enable the config for the Renesas DA9062 PMIC and RTC, as it
> > > > > > is populated on RZ/G2UL SMARC EVK.
> > > > > >
> > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > > >
> > > > > Thanks for your patch!
> > > > >
> > > > > LGTM, but I would have expected (at least)
> > > > >
> > > > >     CONFIG_INPUT_DA9063_ONKEY=m
> > > > >
> > > > > too, cfr. my comments on patch 5/6.
> > > >
> > > > I have tested this, but I get "da9063-onkey da9062-onkey: error -
> ENXIO:
> > > IRQ ONKEY not found"
> > > > So, I won't be able to test onkey events. So plaaning to disable
> > > > this
> > > node.
> > >
> > > Indeed, currently drivers/input/misc/da9063_onkey.c requires an
> > > interrupt to function.  I wonder if it can be made to work using a
> > > timer and pure polling...
> >
> > Yes, that is possible by either adding a debugfs to enable polling for
> testing.
> >
> > Or
> >
> > a device tree property for adding poll delay, so that one can have
> polling support to test onkey driver.
> 
> I would go for the latter, cfr. poll-interval in
> Documentation/devicetree/bindings/input/gpio-keys.yaml.
> 
> > Basically, on a production system we should avoid polling as it
> unnecessarily wakeup the system.
> 
> Wouldn't polling (by Linux) only happen when the system is already awake?

IIUC, That is an overhead. If there is no load, system goes to suspend mode and
the timer for the polling (SCAN keys) will wakeup the system and same 
process repeats. If we have a PMIC IRQ, this won't happen.

> I think the DA9063 itself handles the onkey when the system is asleep.

I will ask HW person to wire IRQ on PMIC, so that I can check the behaviour.

Cheers,
Biju



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 6/6] arm64: defconfig: Enable Renesas DA9062 defconfig

[Geert Uytterhoeven]

Hi Biju,

On Mon, Dec 11, 2023 at 9:32 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > -----Original Message-----
> > From: Geert Uytterhoeven <geert@linux-m68k.org>
> > Sent: Monday, December 11, 2023 8:20 AM
> > Subject: Re: [PATCH 6/6] arm64: defconfig: Enable Renesas DA9062 defconfig
> >
> > On Sun, Dec 10, 2023 at 10:50 AM Biju Das <biju.das.jz@bp.renesas.com>
> > wrote:
> > > > -----Original Message-----
> > > > From: Geert Uytterhoeven <geert@linux-m68k.org>
> > > > Sent: Friday, December 1, 2023 3:42 PM
> > > > Subject: Re: [PATCH 6/6] arm64: defconfig: Enable Renesas DA9062
> > > > defconfig
> > > >
> > > > On Fri, Dec 1, 2023 at 4:02 PM Biju Das <biju.das.jz@bp.renesas.com>
> > > > wrote:
> > > > > > From: Geert Uytterhoeven <geert@linux-m68k.org> On Fri, Dec 1,
> > > > > > 2023 at 12:09 PM Biju Das <biju.das.jz@bp.renesas.com>
> > > > > > wrote:
> > > > > > > Enable the config for the Renesas DA9062 PMIC and RTC, as it
> > > > > > > is populated on RZ/G2UL SMARC EVK.
> > > > > > >
> > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > > > >
> > > > > > Thanks for your patch!
> > > > > >
> > > > > > LGTM, but I would have expected (at least)
> > > > > >
> > > > > >     CONFIG_INPUT_DA9063_ONKEY=m
> > > > > >
> > > > > > too, cfr. my comments on patch 5/6.
> > > > >
> > > > > I have tested this, but I get "da9063-onkey da9062-onkey: error -
> > ENXIO:
> > > > IRQ ONKEY not found"
> > > > > So, I won't be able to test onkey events. So plaaning to disable
> > > > > this
> > > > node.
> > > >
> > > > Indeed, currently drivers/input/misc/da9063_onkey.c requires an
> > > > interrupt to function.  I wonder if it can be made to work using a
> > > > timer and pure polling...
> > >
> > > Yes, that is possible by either adding a debugfs to enable polling for
> > testing.
> > >
> > > Or
> > >
> > > a device tree property for adding poll delay, so that one can have
> > polling support to test onkey driver.
> >
> > I would go for the latter, cfr. poll-interval in
> > Documentation/devicetree/bindings/input/gpio-keys.yaml.
> >
> > > Basically, on a production system we should avoid polling as it
> > unnecessarily wakeup the system.
> >
> > Wouldn't polling (by Linux) only happen when the system is already awake?
>
> IIUC, That is an overhead. If there is no load, system goes to suspend mode and
> the timer for the polling (SCAN keys) will wakeup the system and same
> process repeats. If we have a PMIC IRQ, this won't happen.

Ah, you mean idle. Yes, that does incur a (small) overhead (I guess
there are other timers running, too).
I don't think any timer runs when the system is suspended.

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 6/6] arm64: defconfig: Enable Renesas DA9062 defconfig

[Geert Uytterhoeven]

Hi Biju,

On Mon, Dec 11, 2023 at 9:32 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > -----Original Message-----
> > From: Geert Uytterhoeven <geert@linux-m68k.org>
> > Sent: Monday, December 11, 2023 8:20 AM
> > Subject: Re: [PATCH 6/6] arm64: defconfig: Enable Renesas DA9062 defconfig
> >
> > On Sun, Dec 10, 2023 at 10:50 AM Biju Das <biju.das.jz@bp.renesas.com>
> > wrote:
> > > > -----Original Message-----
> > > > From: Geert Uytterhoeven <geert@linux-m68k.org>
> > > > Sent: Friday, December 1, 2023 3:42 PM
> > > > Subject: Re: [PATCH 6/6] arm64: defconfig: Enable Renesas DA9062
> > > > defconfig
> > > >
> > > > On Fri, Dec 1, 2023 at 4:02 PM Biju Das <biju.das.jz@bp.renesas.com>
> > > > wrote:
> > > > > > From: Geert Uytterhoeven <geert@linux-m68k.org> On Fri, Dec 1,
> > > > > > 2023 at 12:09 PM Biju Das <biju.das.jz@bp.renesas.com>
> > > > > > wrote:
> > > > > > > Enable the config for the Renesas DA9062 PMIC and RTC, as it
> > > > > > > is populated on RZ/G2UL SMARC EVK.
> > > > > > >
> > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > > > >
> > > > > > Thanks for your patch!
> > > > > >
> > > > > > LGTM, but I would have expected (at least)
> > > > > >
> > > > > >     CONFIG_INPUT_DA9063_ONKEY=m
> > > > > >
> > > > > > too, cfr. my comments on patch 5/6.
> > > > >
> > > > > I have tested this, but I get "da9063-onkey da9062-onkey: error -
> > ENXIO:
> > > > IRQ ONKEY not found"
> > > > > So, I won't be able to test onkey events. So plaaning to disable
> > > > > this
> > > > node.
> > > >
> > > > Indeed, currently drivers/input/misc/da9063_onkey.c requires an
> > > > interrupt to function.  I wonder if it can be made to work using a
> > > > timer and pure polling...
> > >
> > > Yes, that is possible by either adding a debugfs to enable polling for
> > testing.
> > >
> > > Or
> > >
> > > a device tree property for adding poll delay, so that one can have
> > polling support to test onkey driver.
> >
> > I would go for the latter, cfr. poll-interval in
> > Documentation/devicetree/bindings/input/gpio-keys.yaml.
> >
> > > Basically, on a production system we should avoid polling as it
> > unnecessarily wakeup the system.
> >
> > Wouldn't polling (by Linux) only happen when the system is already awake?
>
> IIUC, That is an overhead. If there is no load, system goes to suspend mode and
> the timer for the polling (SCAN keys) will wakeup the system and same
> process repeats. If we have a PMIC IRQ, this won't happen.

Ah, you mean idle. Yes, that does incur a (small) overhead (I guess
there are other timers running, too).
I don't think any timer runs when the system is suspended.

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH 6/6] arm64: defconfig: Enable Renesas DA9062 defconfig

[Biju Das]

Hi Geert,

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: Monday, December 11, 2023 8:53 AM
> Subject: Re: [PATCH 6/6] arm64: defconfig: Enable Renesas DA9062 defconfig
> 
> Hi Biju,
> 
> On Mon, Dec 11, 2023 at 9:32 AM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > > -----Original Message-----
> > > From: Geert Uytterhoeven <geert@linux-m68k.org>
> > > Sent: Monday, December 11, 2023 8:20 AM
> > > Subject: Re: [PATCH 6/6] arm64: defconfig: Enable Renesas DA9062
> > > defconfig
> > >
> > > On Sun, Dec 10, 2023 at 10:50 AM Biju Das
> > > <biju.das.jz@bp.renesas.com>
> > > wrote:
> > > > > -----Original Message-----
> > > > > From: Geert Uytterhoeven <geert@linux-m68k.org>
> > > > > Sent: Friday, December 1, 2023 3:42 PM
> > > > > Subject: Re: [PATCH 6/6] arm64: defconfig: Enable Renesas DA9062
> > > > > defconfig
> > > > >
> > > > > On Fri, Dec 1, 2023 at 4:02 PM Biju Das
> > > > > <biju.das.jz@bp.renesas.com>
> > > > > wrote:
> > > > > > > From: Geert Uytterhoeven <geert@linux-m68k.org> On Fri, Dec
> > > > > > > 1,
> > > > > > > 2023 at 12:09 PM Biju Das <biju.das.jz@bp.renesas.com>
> > > > > > > wrote:
> > > > > > > > Enable the config for the Renesas DA9062 PMIC and RTC, as
> > > > > > > > it is populated on RZ/G2UL SMARC EVK.
> > > > > > > >
> > > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > > > > >
> > > > > > > Thanks for your patch!
> > > > > > >
> > > > > > > LGTM, but I would have expected (at least)
> > > > > > >
> > > > > > >     CONFIG_INPUT_DA9063_ONKEY=m
> > > > > > >
> > > > > > > too, cfr. my comments on patch 5/6.
> > > > > >
> > > > > > I have tested this, but I get "da9063-onkey da9062-onkey:
> > > > > > error -
> > > ENXIO:
> > > > > IRQ ONKEY not found"
> > > > > > So, I won't be able to test onkey events. So plaaning to
> > > > > > disable this
> > > > > node.
> > > > >
> > > > > Indeed, currently drivers/input/misc/da9063_onkey.c requires an
> > > > > interrupt to function.  I wonder if it can be made to work using
> > > > > a timer and pure polling...
> > > >
> > > > Yes, that is possible by either adding a debugfs to enable polling
> > > > for
> > > testing.
> > > >
> > > > Or
> > > >
> > > > a device tree property for adding poll delay, so that one can have
> > > polling support to test onkey driver.
> > >
> > > I would go for the latter, cfr. poll-interval in
> > > Documentation/devicetree/bindings/input/gpio-keys.yaml.
> > >
> > > > Basically, on a production system we should avoid polling as it
> > > unnecessarily wakeup the system.
> > >
> > > Wouldn't polling (by Linux) only happen when the system is already
> awake?
> >
> > IIUC, That is an overhead. If there is no load, system goes to suspend
> > mode and the timer for the polling (SCAN keys) will wakeup the system
> > and same process repeats. If we have a PMIC IRQ, this won't happen.
> 
> Ah, you mean idle. Yes, that does incur a (small) overhead (I guess there
> are other timers running, too).

Yes.

> I don't think any timer runs when the system is suspended.

I guess, If we are controlling PM through CPU idle states, this will have an effect.
We schedule timers for poll intervals from onkey driver, when it expires it will wakeup the system
and again based on idle state, it enters into some retention state and goes on.


Cheers,
BIju


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH 6/6] arm64: defconfig: Enable Renesas DA9062 defconfig

[Biju Das]

Hi Geert,

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: Monday, December 11, 2023 8:53 AM
> Subject: Re: [PATCH 6/6] arm64: defconfig: Enable Renesas DA9062 defconfig
> 
> Hi Biju,
> 
> On Mon, Dec 11, 2023 at 9:32 AM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > > -----Original Message-----
> > > From: Geert Uytterhoeven <geert@linux-m68k.org>
> > > Sent: Monday, December 11, 2023 8:20 AM
> > > Subject: Re: [PATCH 6/6] arm64: defconfig: Enable Renesas DA9062
> > > defconfig
> > >
> > > On Sun, Dec 10, 2023 at 10:50 AM Biju Das
> > > <biju.das.jz@bp.renesas.com>
> > > wrote:
> > > > > -----Original Message-----
> > > > > From: Geert Uytterhoeven <geert@linux-m68k.org>
> > > > > Sent: Friday, December 1, 2023 3:42 PM
> > > > > Subject: Re: [PATCH 6/6] arm64: defconfig: Enable Renesas DA9062
> > > > > defconfig
> > > > >
> > > > > On Fri, Dec 1, 2023 at 4:02 PM Biju Das
> > > > > <biju.das.jz@bp.renesas.com>
> > > > > wrote:
> > > > > > > From: Geert Uytterhoeven <geert@linux-m68k.org> On Fri, Dec
> > > > > > > 1,
> > > > > > > 2023 at 12:09 PM Biju Das <biju.das.jz@bp.renesas.com>
> > > > > > > wrote:
> > > > > > > > Enable the config for the Renesas DA9062 PMIC and RTC, as
> > > > > > > > it is populated on RZ/G2UL SMARC EVK.
> > > > > > > >
> > > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > > > > >
> > > > > > > Thanks for your patch!
> > > > > > >
> > > > > > > LGTM, but I would have expected (at least)
> > > > > > >
> > > > > > >     CONFIG_INPUT_DA9063_ONKEY=m
> > > > > > >
> > > > > > > too, cfr. my comments on patch 5/6.
> > > > > >
> > > > > > I have tested this, but I get "da9063-onkey da9062-onkey:
> > > > > > error -
> > > ENXIO:
> > > > > IRQ ONKEY not found"
> > > > > > So, I won't be able to test onkey events. So plaaning to
> > > > > > disable this
> > > > > node.
> > > > >
> > > > > Indeed, currently drivers/input/misc/da9063_onkey.c requires an
> > > > > interrupt to function.  I wonder if it can be made to work using
> > > > > a timer and pure polling...
> > > >
> > > > Yes, that is possible by either adding a debugfs to enable polling
> > > > for
> > > testing.
> > > >
> > > > Or
> > > >
> > > > a device tree property for adding poll delay, so that one can have
> > > polling support to test onkey driver.
> > >
> > > I would go for the latter, cfr. poll-interval in
> > > Documentation/devicetree/bindings/input/gpio-keys.yaml.
> > >
> > > > Basically, on a production system we should avoid polling as it
> > > unnecessarily wakeup the system.
> > >
> > > Wouldn't polling (by Linux) only happen when the system is already
> awake?
> >
> > IIUC, That is an overhead. If there is no load, system goes to suspend
> > mode and the timer for the polling (SCAN keys) will wakeup the system
> > and same process repeats. If we have a PMIC IRQ, this won't happen.
> 
> Ah, you mean idle. Yes, that does incur a (small) overhead (I guess there
> are other timers running, too).

Yes.

> I don't think any timer runs when the system is suspended.

I guess, If we are controlling PM through CPU idle states, this will have an effect.
We schedule timers for poll intervals from onkey driver, when it expires it will wakeup the system
and again based on idle state, it enters into some retention state and goes on.


Cheers,
BIju