linux-rtc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: Biju Das <biju.das.jz@bp.renesas.com>,
	Alessandro Zummo <a.zummo@towertech.it>,
	Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: John Stultz <jstultz@google.com>, Stephen Boyd <sboyd@kernel.org>,
	Douglas Anderson <dianders@chromium.org>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Biju Das <biju.das.au@gmail.com>,
	"linux-rtc@vger.kernel.org" <linux-rtc@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-renesas-soc@vger.kernel.org"
	<linux-renesas-soc@vger.kernel.org>
Subject: RE: [PATCH v2] alarmtimer: Fix rebind failure
Date: Tue, 10 Oct 2023 00:00:12 +0200	[thread overview]
Message-ID: <87bkd7pic3.ffs@tglx> (raw)
In-Reply-To: <TYCPR01MB11269C6BF3934F9AAC44F855186CEA@TYCPR01MB11269.jpnprd01.prod.outlook.com>

On Mon, Oct 09 2023 at 17:02, Biju Das wrote:
>> On Mon, Oct 09 2023 at 15:30, Biju Das wrote:
>> > You mean we should update[1] (charger-manager driver)as it is the one
>> > using alarmtimer_get_rtcdev()??
>> 
>> # git grep -c alarmtimer_get_rtcdev
>> drivers/power/supply/charger-manager.c:1
>> include/linux/alarmtimer.h:2
>> kernel/time/alarmtimer.c:10
>
> kernel/time/alarmtimer.c has alarmtimer_get_rtcdev()check everywhere,
> that is missing in charger-manager.c. I will add the same, is it ok?

The code does in the init function:

      if (alarmtimer_get_rtcdev()) {
         ....
      }

IOW, charger-manager.c expects that alarm is working when
alarmtimer_get_rtcdev() returns non NULL at init. So ripping the RTC
device out under it is going to result in a disfunctional driver. I'm
not convinced that you can fix this by sprinkling a ton of checks around
the code.

But that's not the worst of it. The alarmtimer infrastructure is
generally not designed for device/module removal. Why?

The posix timer interface is fundamentally expecting that an armed alarm
timer is actually functional. The fact that the class interface does not
have a remove_dev callback is not an oversight and holding a reference
on the module and a reference on the device is intended to ensure that
the device cannot vanish.

The changelog lacks any form of explanation why this is required and how
removal of the registered RTC device is actually possible. Neither does
it provide any analysis why this cannot result in malfunction.

Thanks,

        tglx












  reply	other threads:[~2023-10-09 22:00 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-22  8:12 [PATCH v2] alarmtimer: Fix rebind failure Biju Das
2023-10-09  7:00 ` Biju Das
2023-10-09 15:10 ` Thomas Gleixner
2023-10-09 15:30   ` Biju Das
2023-10-09 15:46     ` Thomas Gleixner
2023-10-09 17:02       ` Biju Das
2023-10-09 22:00         ` Thomas Gleixner [this message]
2023-10-10  6:18           ` Biju Das
2023-10-10 15:16             ` Thomas Gleixner
2023-10-10 15:45               ` Geert Uytterhoeven
2023-10-11  6:58                 ` Biju Das
2023-10-11  9:12                   ` Thomas Gleixner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87bkd7pic3.ffs@tglx \
    --to=tglx@linutronix.de \
    --cc=a.zummo@towertech.it \
    --cc=alexandre.belloni@bootlin.com \
    --cc=biju.das.au@gmail.com \
    --cc=biju.das.jz@bp.renesas.com \
    --cc=dianders@chromium.org \
    --cc=geert+renesas@glider.be \
    --cc=jstultz@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=linux-rtc@vger.kernel.org \
    --cc=sboyd@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).