Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:4 as permitted sender) client-ip=2620:137:e000::3:4;
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
In-Reply-To: <TYCPR01MB11269C6BF3934F9AAC44F855186CEA@TYCPR01MB11269.jpnprd01.prod.outlook.com>
References: <20230922081208.26334-1-biju.das.jz@bp.renesas.com>
 <87il7fq1al.ffs@tglx>
 <TYCPR01MB112697A5D4B57101CDE27C88D86CEA@TYCPR01MB11269.jpnprd01.prod.outlook.com>
 <87fs2jpznr.ffs@tglx>
 <TYCPR01MB11269C6BF3934F9AAC44F855186CEA@TYCPR01MB11269.jpnprd01.prod.outlook.com>
Date:   Tue, 10 Oct 2023 00:00:12 +0200
Message-ID: <87bkd7pic3.ffs@tglx>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

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