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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Geert,

Thanks for the feedback.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> 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@xxxxxxxxxxxxxx>
> wrote:
> > Replace dev_err()->dev_err_probe() to simpilfy probe().
> >
> > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> 
> 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@xxxxxxxxx>
> 
> 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




[Index of Archives]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux