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

From: Biju Das <biju.das.jz@bp.renesas.com>
To: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>,
	Alessandro Zummo <a.zummo@towertech.it>,
	Support Opensource <support.opensource@diasemi.com>,
	"linux-rtc@vger.kernel.org" <linux-rtc@vger.kernel.org>,
	Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>,
	biju.das.au <biju.das.au@gmail.com>,
	"linux-renesas-soc@vger.kernel.org"
	<linux-renesas-soc@vger.kernel.org>
Subject: RE: [PATCH 3/6] rtc: da9063: Use dev_err_probe()
Date: Fri, 1 Dec 2023 15:43:27 +0000	[thread overview]
Message-ID: <TYCPR01MB1126992FDD7B1F2DEFEFD8BE68681A@TYCPR01MB11269.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <202312011520103c5f28dc@mail.local>

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