Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
MIME-Version: 1.0
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>
 <87bkd7pic3.ffs@tglx> <TYCPR01MB11269FF2DBFDC96B9C12D2E5E86CDA@TYCPR01MB11269.jpnprd01.prod.outlook.com>
 <87o7h6o6d1.ffs@tglx>
In-Reply-To: <87o7h6o6d1.ffs@tglx>
From:   Geert Uytterhoeven <geert@linux-m68k.org>
Date:   Tue, 10 Oct 2023 17:45:46 +0200
Message-ID: <CAMuHMdVJnqkF7xmjfOyoxE_Lq=AO85CDD85qu3O+xcSr-3BLTQ@mail.gmail.com>
Subject: Re: [PATCH v2] alarmtimer: Fix rebind failure
To:     Thomas Gleixner <tglx@linutronix.de>
Cc:     Biju Das <biju.das.jz@bp.renesas.com>,
        Alessandro Zummo <a.zummo@towertech.it>,
        Alexandre Belloni <alexandre.belloni@bootlin.com>,
        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>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8BIT
Precedence: bulk

Hi Thomas,

On Tue, Oct 10, 2023 at 5:16 PM Thomas Gleixner <tglx@linutronix.de> wrote:
> On Tue, Oct 10 2023 at 06:18, Biju Das wrote:
> > RTC driver is defined as a module, so I was testing
> > remove/unbind followed by install/bind on RTC driver to check
> > any resource leakage and found that device is not working properly.
> >
> > As you mentioned above, we should not remove RTC driver. So I would
> > like to drop this patch.
> >
> > Is there any place we can document this to avoid another person doing
> > same mistake?
>
> The point is that the removal should not happen in the first place.
>
> Though it seems that even a held refcount on the module is not
> preventing that, which is bad to begin with.

Indeed.  I had expected to find at least one RTC driver for a device
on a hot-pluggable bus like USB, but apparently we have none of these
(yet).  So currently RTC device removal can only be triggered by a
manual sysfs unbind or delete_device.

> That aside I'm not saying that supporting removal is completely
> impossible. The charger driver can probably be fixed, but as this is a
> user space visible change this needs a lot of thoughts and proper
> analysis why such a change would be correct under all circumstances.

The charger manager seems to be considered a legacy driver.
Devices are only instantiated from the drivers/mfd/88pm860x.c (as used
on Marvell PXA910 DKB boards), or directly from DT (no upstream
users). The DT bindings say:

    Binding for the legacy charger manager driver.
    Please do not use for new products.

The "if (alarmtimer_get_rtcdev()) { ... }" you pointed out in the
probe function  seems to be rather fragile, as it depends on probe
order. And both CHARGER_MANAGER and RTC_DRV_88PM860X can be modular.

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