RE: [PATCH v2] alarmtimer: Fix rebind failure
From: Biju Das
Date: Wed Oct 11 2023 - 02:58:11 EST
Hi Geert and Thomas,
> Subject: Re: [PATCH v2] alarmtimer: Fix rebind failure
>
> Hi Thomas,
>
> On Tue, Oct 10, 2023 at 5:16 PM Thomas Gleixner <tglx@xxxxxxxxxxxxx> 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.
Does it mean that current patch is fine? On normal scenario,
no one will remove RTC device, so nothing to worry about battery charger. On exceptional cases if anyone wants to remove RTC driver, this patch will help(for eg: checking resource leak remove/unbind followed by modprobe/bind).
Cheers,
Biju