Re: [PATCH] fbdev: Statically link the framebuffer notification functions

["Antonino A. Daplas" <adaplas@gmail.com>]

Jon Smirl wrote:
> On 7/11/06, Antonino A. Daplas <adaplas@gmail.com> wrote:
>> Jon Smirl wrote:
>> > On 7/11/06, Antonino A. Daplas <adaplas@gmail.com> wrote:
>> >> Jon Smirl wrote:
>> >> > On 7/11/06, Antonino A. Daplas <adaplas@gmail.com> wrote:
>> >> >> The backlight and lcd subsystems can be notified by the framebuffer
>> >> layer
>> >> >> of blanking events.  However, these subsystems, as a whole, can
>> >> function
>> >> >> independently from the framebuffer layer. But in order to enable to
>> >> >> the lcd and backlight subsystems, the framebuffer has to be
>> compiled
>> >> >> also,
>> >> >> effectively sucking in a huge amount of unneeded code. Besides, the
>> >> >> dependency
>> >> >> is introducing a lot of compilation problems.
>> >> >
>> >> > This code is effectively rebuilding a fb specific version of
>> >> > inter_module_get/put., something that was removed earlier.
>> >>
>> >> Huh? I don't see any semblance of inter_module_* or symbol_* in there.
>> >> Read the patch again.
>> >
>> > You are providing a fixed point to do a rendezvous between modules
>> > without refcount tracking. That's what inter_module did.
>>
>> No, you're confused on inter_module. inter_module_* allows 2 or more
>> modules to share data. The danger is that one module may disappear
>> while the other is still accessing the data.
>>
>> In this case, there is absolutely no data sharing. One module can
>> safely unload without affecting the other. The only danger is
>> that one might be in a midst of a calling the callout function while
>> the other is unregistering its notifier block. But then the notifier
>> chain already protects this from happening.
> 
> The code looks ok but this sure smells like inter_module_*.

I assure you, there is no smell of inter_module_* here. What scenario
are you afraid of?

> I guess
> inter_module had to deal with arbitrary users and this code is dealing
> with a fixed set of clients which makes it more manageable.
> 
> Have you considered making this a generic service and not fb specific?
> 

It's basically a wrapper to the notifier_call_chain, that's as generic
as it can get. And yes, it's not fb_specific (meaning, there's no need
for the client module to know fbdev internals), that's why the lcd and
backlight subsystem can take advantage of it. 

Tony