public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* Depmod errors on 2.6.17.4/2.6.18-rc1/2.6.18-rc1-mm1
@ 2006-07-10 12:32 Matt Reuther
  2006-07-10 13:02 ` Antonino A. Daplas
  0 siblings, 1 reply; 26+ messages in thread
From: Matt Reuther @ 2006-07-10 12:32 UTC (permalink / raw)
  To: Andrew Morton, LKML

[-- Attachment #1: Type: text/plain, Size: 1243 bytes --]

The following errors show up on 'make modules_install' for the 2.6.18-rc1-mm1 
kernel. The snd-miro ones have actually been there since at least 2.6.17.4, 
and they show up in 2.6.18-rc1 as well.

  INSTALL sound/usb/snd-usb-audio.ko
  INSTALL sound/usb/snd-usb-lib.ko
  INSTALL sound/usb/usx2y/snd-usb-usx2y.ko
if [ -r System.map -a -x /sbin/depmod ]; then /sbin/depmod -ae -F System.map  
2.6.18-rc1-mm1; fi
WARNING: /lib/modules/2.6.18-rc1-mm1/kernel/sound/isa/opti9xx/snd-miro.ko 
needs unknown symbol snd_cs4231_create
WARNING: /lib/modules/2.6.18-rc1-mm1/kernel/sound/isa/opti9xx/snd-miro.ko 
needs unknown symbol snd_cs4231_pcm
WARNING: /lib/modules/2.6.18-rc1-mm1/kernel/sound/isa/opti9xx/snd-miro.ko 
needs unknown symbol snd_cs4231_timer
WARNING: /lib/modules/2.6.18-rc1-mm1/kernel/sound/isa/opti9xx/snd-miro.ko 
needs unknown symbol snd_cs4231_mixer
WARNING: /lib/modules/2.6.18-rc1-mm1/kernel/fs/reiser4/reiser4.ko needs 
unknown symbol generic_file_read
WARNING: /lib/modules/2.6.18-rc1-mm1/kernel/drivers/video/backlight/backlight.ko 
needs unknown symbol fb_unregister_client
WARNING: /lib/modules/2.6.18-rc1-mm1/kernel/drivers/video/backlight/backlight.ko 
needs unknown symbol fb_register_client

My config is attached.

Matt

[-- Attachment #2: config-2.6.18-rc1-mm1-1.bz2 --]
[-- Type: application/x-bzip2, Size: 14315 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: Depmod errors on 2.6.17.4/2.6.18-rc1/2.6.18-rc1-mm1
  2006-07-10 12:32 Depmod errors on 2.6.17.4/2.6.18-rc1/2.6.18-rc1-mm1 Matt Reuther
@ 2006-07-10 13:02 ` Antonino A. Daplas
  2006-07-10 15:32   ` Matt Reuther
  0 siblings, 1 reply; 26+ messages in thread
From: Antonino A. Daplas @ 2006-07-10 13:02 UTC (permalink / raw)
  To: Matt Reuther; +Cc: Andrew Morton, LKML

Matt Reuther wrote:
> The following errors show up on 'make modules_install' for the 2.6.18-rc1-mm1 
> kernel. The snd-miro ones have actually been there since at least 2.6.17.4, 
> and they show up in 2.6.18-rc1 as well.

> WARNING: /lib/modules/2.6.18-rc1-mm1/kernel/drivers/video/backlight/backlight.ko 
> needs unknown symbol fb_unregister_client
> WARNING: /lib/modules/2.6.18-rc1-mm1/kernel/drivers/video/backlight/backlight.ko 
> needs unknown symbol fb_register_client
> 

CONFIG_FB=n, CONFIG_BACKLIGHT_CLASS_DEVICE=m should not be possible in
2.6.18-rc1-mm1 and 2.6.18-rc1.  Can you run kconfig again?

Tony


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: Depmod errors on 2.6.17.4/2.6.18-rc1/2.6.18-rc1-mm1
  2006-07-10 13:02 ` Antonino A. Daplas
@ 2006-07-10 15:32   ` Matt Reuther
  2006-07-10 15:58     ` Antonino A. Daplas
  0 siblings, 1 reply; 26+ messages in thread
From: Matt Reuther @ 2006-07-10 15:32 UTC (permalink / raw)
  To: Antonino A. Daplas; +Cc: Andrew Morton, LKML

Quoting "Antonino A. Daplas" <adaplas@gmail.com>:

> Matt Reuther wrote:
>> The following errors show up on 'make modules_install' for the 
>> 2.6.18-rc1-mm1
>> kernel. The snd-miro ones have actually been there since at least 2.6.17.4,
>> and they show up in 2.6.18-rc1 as well.
>
>> WARNING: 
>> /lib/modules/2.6.18-rc1-mm1/kernel/drivers/video/backlight/backlight.ko
>> needs unknown symbol fb_unregister_client
>> WARNING: 
>> /lib/modules/2.6.18-rc1-mm1/kernel/drivers/video/backlight/backlight.ko
>> needs unknown symbol fb_register_client
>>
>
> CONFIG_FB=n, CONFIG_BACKLIGHT_CLASS_DEVICE=m should not be possible in
> 2.6.18-rc1-mm1 and 2.6.18-rc1.  Can you run kconfig again?
>
> Tony

I am not at the computer right now, but I will try later.

Here is how I got config-2.6.18-rc1-mm1. I copied this config from 
2.6.18-rc1, which I had created with 'make menuconfig'. I ran 'make 
oldconfig' on the config-2.6.18-rc1 and answered the new questions to 
generate config-2.6.18-rc1-mm1. I compiled it from there. Does 'make 
oldconfig' not work properly anymore?

Matt

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: Depmod errors on 2.6.17.4/2.6.18-rc1/2.6.18-rc1-mm1
  2006-07-10 15:32   ` Matt Reuther
@ 2006-07-10 15:58     ` Antonino A. Daplas
  2006-07-11  3:27       ` Matt Reuther
  0 siblings, 1 reply; 26+ messages in thread
From: Antonino A. Daplas @ 2006-07-10 15:58 UTC (permalink / raw)
  To: Matt Reuther; +Cc: Andrew Morton, LKML

Matt Reuther wrote:
> Quoting "Antonino A. Daplas" <adaplas@gmail.com>:
> 
>> Matt Reuther wrote:
>>> The following errors show up on 'make modules_install' for the
>>> 2.6.18-rc1-mm1
>>> kernel. The snd-miro ones have actually been there since at least
>>> 2.6.17.4,
>>> and they show up in 2.6.18-rc1 as well.
>>
>>> WARNING:
>>> /lib/modules/2.6.18-rc1-mm1/kernel/drivers/video/backlight/backlight.ko
>>> needs unknown symbol fb_unregister_client
>>> WARNING:
>>> /lib/modules/2.6.18-rc1-mm1/kernel/drivers/video/backlight/backlight.ko
>>> needs unknown symbol fb_register_client
>>>
>>
>> CONFIG_FB=n, CONFIG_BACKLIGHT_CLASS_DEVICE=m should not be possible in
>> 2.6.18-rc1-mm1 and 2.6.18-rc1.  Can you run kconfig again?
>>
>> Tony
> 
> I am not at the computer right now, but I will try later.
> 
> Here is how I got config-2.6.18-rc1-mm1. I copied this config from
> 2.6.18-rc1, which I had created with 'make menuconfig'. I ran 'make
> oldconfig' on the config-2.6.18-rc1 and answered the new questions to
> generate config-2.6.18-rc1-mm1. I compiled it from there. Does 'make
> oldconfig' not work properly anymore?

I really don't know.  I have received several bug reports where the
main cause was that a kconfig option changed after upgrading kernels.

I tested with make menuconfig, and it's not possible to set lcd/backlight
options if CONFIG_FB is not set.

Tony

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: Depmod errors on 2.6.17.4/2.6.18-rc1/2.6.18-rc1-mm1
  2006-07-10 15:58     ` Antonino A. Daplas
@ 2006-07-11  3:27       ` Matt Reuther
  2006-07-11  4:52         ` [PATCH] appledisplay/backlight (Depmod errors on 2.6.17.4/2.6.18-rc1/2.6.18-rc1-mm1) Randy.Dunlap
                           ` (2 more replies)
  0 siblings, 3 replies; 26+ messages in thread
From: Matt Reuther @ 2006-07-11  3:27 UTC (permalink / raw)
  To: Antonino A. Daplas; +Cc: Andrew Morton, LKML

[-- Attachment #1: Type: text/plain, Size: 2441 bytes --]

On Monday 10 July 2006 11:58 am, Antonino A. Daplas wrote:
> Matt Reuther wrote:
> > Quoting "Antonino A. Daplas" <adaplas@gmail.com>:
> >>
> >> CONFIG_FB=n, CONFIG_BACKLIGHT_CLASS_DEVICE=m should not be possible in
> >> 2.6.18-rc1-mm1 and 2.6.18-rc1.  Can you run kconfig again?
> >>
> >> Tony
> >
> > I am not at the computer right now, but I will try later.
> >
> > Here is how I got config-2.6.18-rc1-mm1. I copied this config from
> > 2.6.18-rc1, which I had created with 'make menuconfig'. I ran 'make
> > oldconfig' on the config-2.6.18-rc1 and answered the new questions to
> > generate config-2.6.18-rc1-mm1. I compiled it from there. Does 'make
> > oldconfig' not work properly anymore?
>
> I really don't know.  I have received several bug reports where the
> main cause was that a kconfig option changed after upgrading kernels.
>
> I tested with make menuconfig, and it's not possible to set lcd/backlight
> options if CONFIG_FB is not set.
>
> Tony

I ran 'make menuconfig' and I got the same warnings. Perhaps CONFIG_FB needs 
to be part of the 'selects' line for any option that selects the backlight 
support. I think the USB Apple Cinema display support, which I set as 
modular, might have selected backlight. I don't need framebuffer support, so 
I have that shut off. Here are the depmod warnings once again:

  INSTALL sound/usb/snd-usb-audio.ko
  INSTALL sound/usb/snd-usb-lib.ko
  INSTALL sound/usb/usx2y/snd-usb-usx2y.ko
if [ -r System.map -a -x /sbin/depmod ]; then /sbin/depmod -ae -F System.map  
2.6.18-rc1-mm1; fi
WARNING: /lib/modules/2.6.18-rc1-mm1/kernel/sound/isa/opti9xx/snd-miro.ko 
needs unknown symbol snd_cs4231_create
WARNING: /lib/modules/2.6.18-rc1-mm1/kernel/sound/isa/opti9xx/snd-miro.ko 
needs unknown symbol snd_cs4231_pcm
WARNING: /lib/modules/2.6.18-rc1-mm1/kernel/sound/isa/opti9xx/snd-miro.ko 
needs unknown symbol snd_cs4231_timer
WARNING: /lib/modules/2.6.18-rc1-mm1/kernel/sound/isa/opti9xx/snd-miro.ko 
needs unknown symbol snd_cs4231_mixer
WARNING: /lib/modules/2.6.18-rc1-mm1/kernel/fs/reiser4/reiser4.ko needs 
unknown symbol generic_file_read
WARNING: /lib/modules/2.6.18-rc1-mm1/kernel/drivers/video/backlight/backlight.ko 
needs unknown symbol fb_unregister_client
WARNING: /lib/modules/2.6.18-rc1-mm1/kernel/drivers/video/backlight/backlight.ko 
needs unknown symbol fb_register_client

I ran make menuconfig with the config file I sent before. The new config is 
attached.

Matt

[-- Attachment #2: config-2.6.18-rc1-mm1-2.bz2 --]
[-- Type: application/x-bzip2, Size: 14316 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH] appledisplay/backlight (Depmod errors on 2.6.17.4/2.6.18-rc1/2.6.18-rc1-mm1)
  2006-07-11  3:27       ` Matt Reuther
@ 2006-07-11  4:52         ` Randy.Dunlap
  2006-07-11  6:54           ` [PATCH] backlight: lcd: Remove dependency from the framebuffer layer Antonino A. Daplas
  2006-07-11  7:04           ` Antonino A. Daplas
  2006-07-11  5:16         ` [PATCH] sound-miro unknown symbols (Depmod errors on 2.6.17.4/2.6.18-rc1/2.6.18-rc1-mm1) Randy.Dunlap
  2006-07-11  6:56         ` Depmod errors on 2.6.17.4/2.6.18-rc1/2.6.18-rc1-mm1 Antonino A. Daplas
  2 siblings, 2 replies; 26+ messages in thread
From: Randy.Dunlap @ 2006-07-11  4:52 UTC (permalink / raw)
  To: Matt Reuther; +Cc: adaplas, akpm, lkml

> > >> CONFIG_FB=n, CONFIG_BACKLIGHT_CLASS_DEVICE=m should not be possible in
> > >> 2.6.18-rc1-mm1 and 2.6.18-rc1.  Can you run kconfig again?
> > >>
> > >> Tony
> >
> > I tested with make menuconfig, and it's not possible to set lcd/backlight
> > options if CONFIG_FB is not set.

Tony, it is possiblle when the USB APPLEDISPLAY option is set and it selects
backlight options without regard for FB.
 
> WARNING: /lib/modules/2.6.18-rc1-mm1/kernel/drivers/video/backlight/backlight.ko 
> needs unknown symbol fb_unregister_client
> WARNING: /lib/modules/2.6.18-rc1-mm1/kernel/drivers/video/backlight/backlight.ko 
> needs unknown symbol fb_register_client

This patch fixes it for me.  Is it the right thing to do?
---

From: Randy Dunlap <rdunlap@xenotime.net>

appledisplay calls fb_register_client() so needs to depend on FB.

Signed-off-by: Randy Dunlap <rdunlap@xenotime.net>
---
 drivers/usb/misc/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- linux-2618-rc1mm1.orig/drivers/usb/misc/Kconfig
+++ linux-2618-rc1mm1/drivers/usb/misc/Kconfig
@@ -163,7 +163,7 @@ config USB_IDMOUSE
 
 config USB_APPLEDISPLAY
 	tristate "Apple Cinema Display support"
-	depends on USB
+	depends on USB && FB
 	select BACKLIGHT_LCD_SUPPORT
 	select BACKLIGHT_CLASS_DEVICE
 	help


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH] sound-miro unknown symbols (Depmod errors on 2.6.17.4/2.6.18-rc1/2.6.18-rc1-mm1)
  2006-07-11  3:27       ` Matt Reuther
  2006-07-11  4:52         ` [PATCH] appledisplay/backlight (Depmod errors on 2.6.17.4/2.6.18-rc1/2.6.18-rc1-mm1) Randy.Dunlap
@ 2006-07-11  5:16         ` Randy.Dunlap
  2006-07-11  6:56         ` Depmod errors on 2.6.17.4/2.6.18-rc1/2.6.18-rc1-mm1 Antonino A. Daplas
  2 siblings, 0 replies; 26+ messages in thread
From: Randy.Dunlap @ 2006-07-11  5:16 UTC (permalink / raw)
  To: Matt Reuther, perex; +Cc: akpm, linux-kernel

> WARNING: /lib/modules/2.6.18-rc1-mm1/kernel/sound/isa/opti9xx/snd-miro.ko 
> needs unknown symbol snd_cs4231_create
> WARNING: /lib/modules/2.6.18-rc1-mm1/kernel/sound/isa/opti9xx/snd-miro.ko 
> needs unknown symbol snd_cs4231_pcm
> WARNING: /lib/modules/2.6.18-rc1-mm1/kernel/sound/isa/opti9xx/snd-miro.ko 
> needs unknown symbol snd_cs4231_timer
> WARNING: /lib/modules/2.6.18-rc1-mm1/kernel/sound/isa/opti9xx/snd-miro.ko 
> needs unknown symbol snd_cs4231_mixer
> WARNING: /lib/modules/2.6.18-rc1-mm1/kernel/fs/reiser4/reiser4.ko needs 
> unknown symbol generic_file_read

Someone posted a patch for the reiser4 function-name conversion.

Here is a patch for the snd-miro driver references.

---
From: Randy Dunlap <rdunlap@xenotime.net>

Fix undefined (missing) references in ISA MIRO sound driver.

Signed-off-by: Randy Dunlap <rdunlap@xenotime.net>
---
 sound/isa/cs423x/Makefile |    1 +
 1 file changed, 1 insertion(+)

--- linux-2618-rc1mm1.orig/sound/isa/cs423x/Makefile
+++ linux-2618-rc1mm1/sound/isa/cs423x/Makefile
@@ -11,6 +11,7 @@ snd-cs4236-objs := cs4236.o
 
 # Toplevel Module Dependency
 obj-$(CONFIG_SND_AZT2320) += snd-cs4231-lib.o
+obj-$(CONFIG_SND_MIRO) += snd-cs4231-lib.o
 obj-$(CONFIG_SND_OPL3SA2) += snd-cs4231-lib.o
 obj-$(CONFIG_SND_CS4231) += snd-cs4231.o snd-cs4231-lib.o
 obj-$(CONFIG_SND_CS4232) += snd-cs4232.o snd-cs4231-lib.o

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH] backlight: lcd: Remove dependency from the framebuffer layer
  2006-07-11  4:52         ` [PATCH] appledisplay/backlight (Depmod errors on 2.6.17.4/2.6.18-rc1/2.6.18-rc1-mm1) Randy.Dunlap
@ 2006-07-11  6:54           ` Antonino A. Daplas
  2006-07-11  7:04           ` Antonino A. Daplas
  1 sibling, 0 replies; 26+ messages in thread
From: Antonino A. Daplas @ 2006-07-11  6:54 UTC (permalink / raw)
  To: Randy.Dunlap; +Cc: Matt Reuther, akpm, lkml, Andrew Zabolotny

The backlight and lcd layer should be independent from the framebuffer layer.
It can use the services offered by the framebuffer, but its absence should
not prevent the backlight/lcd layer from functioning.

Signed-off-by: Antonino Daplas <adaplas@pol.net>
---

Randy.Dunlap wrote:
>>>>> CONFIG_FB=n, CONFIG_BACKLIGHT_CLASS_DEVICE=m should not be possible in
>>>>> 2.6.18-rc1-mm1 and 2.6.18-rc1.  Can you run kconfig again?
>>>>>
>>>>> Tony
>>> I tested with make menuconfig, and it's not possible to set lcd/backlight
>>> options if CONFIG_FB is not set.
> 
> Tony, it is possiblle when the USB APPLEDISPLAY option is set and it selects
> backlight options without regard for FB.
>  
>> WARNING: /lib/modules/2.6.18-rc1-mm1/kernel/drivers/video/backlight/backlight.ko 
>> needs unknown symbol fb_unregister_client
>> WARNING: /lib/modules/2.6.18-rc1-mm1/kernel/drivers/video/backlight/backlight.ko 
>> needs unknown symbol fb_register_client
> 
> This patch fixes it for me.  Is it the right thing to do?

In this case, yes.  But I think the better solution is to eliminate
framebuffer dependency.

Tony

 drivers/video/Kconfig               |    2 -
 drivers/video/backlight/Kconfig     |    4 +-
 drivers/video/backlight/backlight.c |   85 ++++++++++++++++++++++-------------
 drivers/video/backlight/lcd.c       |   76 +++++++++++++++++++------------
 4 files changed, 103 insertions(+), 64 deletions(-)

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index c51a54b..9d9d02a 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -1622,7 +1622,7 @@ if FB || SGI_NEWPORT_CONSOLE
 	source "drivers/video/logo/Kconfig"
 endif
 
-if FB && SYSFS
+if SYSFS
 	source "drivers/video/backlight/Kconfig"
 endif
 
diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index 022f9d3..02f1529 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -10,7 +10,7 @@ menuconfig BACKLIGHT_LCD_SUPPORT
 
 config BACKLIGHT_CLASS_DEVICE
         tristate "Lowlevel Backlight controls"
-	depends on BACKLIGHT_LCD_SUPPORT && FB
+	depends on BACKLIGHT_LCD_SUPPORT
 	default m
 	help
 	  This framework adds support for low-level control of the LCD
@@ -26,7 +26,7 @@ config BACKLIGHT_DEVICE
 
 config LCD_CLASS_DEVICE
         tristate "Lowlevel LCD controls"
-	depends on BACKLIGHT_LCD_SUPPORT && FB
+	depends on BACKLIGHT_LCD_SUPPORT
 	default m
 	help
 	  This framework adds support for low-level control of LCD.
diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index 27597c5..fd22a7c 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -14,6 +14,57 @@ #include <linux/ctype.h>
 #include <linux/err.h>
 #include <linux/fb.h>
 
+
+#if defined(CONFIG_FB) || (defined(CONFIG_FB_MODULE) && \
+			   defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE))
+/* This callback gets called when something important happens inside a
+ * framebuffer driver. We're looking if that important event is blanking,
+ * and if it is, we're switching backlight power as well ...
+ */
+static int __deprecated fb_notifier_callback(struct notifier_block *self,
+				unsigned long event, void *data)
+{
+	struct backlight_device *bd;
+	struct fb_event *evdata =(struct fb_event *)data;
+
+	/* If we aren't interested in this event, skip it immediately ... */
+	if (event != FB_EVENT_BLANK)
+		return 0;
+
+	bd = container_of(self, struct backlight_device, fb_notif);
+	down(&bd->sem);
+	if (bd->props)
+		if (!bd->props->check_fb ||
+		    bd->props->check_fb(evdata->info)) {
+			bd->props->fb_blank = *(int *)evdata->data;
+			if (likely(bd->props && bd->props->update_status))
+				bd->props->update_status(bd);
+		}
+	up(&bd->sem);
+	return 0;
+}
+
+static int backlight_register_fb(struct backlight_device *bd)
+{
+	memset(&bd->fb_notif, 0, sizeof(bd->fb_notif));
+	bd->fb_notif.notifier_call = fb_notifier_callback;
+
+	return fb_register_client(&bd->fb_notif);
+}
+
+static void backlight_unregister_fb(struct backlight_device *bd)
+{
+	fb_unregister_client(&bd->fb_notif);
+}
+#else
+static inline int backlight_register_fb(struct backlight_device *bd)
+{
+	return 0;
+}
+
+#define backlight_unregister_fb(...) do { } while (0)
+#endif /* CONFIG_FB */
+
 static ssize_t backlight_show_power(struct class_device *cdev, char *buf)
 {
 	int rc = -ENXIO;
@@ -151,33 +202,6 @@ static struct class_device_attribute bl_
 	DECLARE_ATTR(max_brightness, 0444, backlight_show_max_brightness, NULL),
 };
 
-/* This callback gets called when something important happens inside a
- * framebuffer driver. We're looking if that important event is blanking,
- * and if it is, we're switching backlight power as well ...
- */
-static int fb_notifier_callback(struct notifier_block *self,
-				unsigned long event, void *data)
-{
-	struct backlight_device *bd;
-	struct fb_event *evdata =(struct fb_event *)data;
-
-	/* If we aren't interested in this event, skip it immediately ... */
-	if (event != FB_EVENT_BLANK)
-		return 0;
-
-	bd = container_of(self, struct backlight_device, fb_notif);
-	down(&bd->sem);
-	if (bd->props)
-		if (!bd->props->check_fb ||
-		    bd->props->check_fb(evdata->info)) {
-			bd->props->fb_blank = *(int *)evdata->data;
-			if (likely(bd->props && bd->props->update_status))
-				bd->props->update_status(bd);
-		}
-	up(&bd->sem);
-	return 0;
-}
-
 /**
  * backlight_device_register - create and register a new object of
  *   backlight_device class.
@@ -215,10 +239,7 @@ error:		kfree(new_bd);
 		return ERR_PTR(rc);
 	}
 
-	memset(&new_bd->fb_notif, 0, sizeof(new_bd->fb_notif));
-	new_bd->fb_notif.notifier_call = fb_notifier_callback;
-
-	rc = fb_register_client(&new_bd->fb_notif);
+	rc = backlight_register_fb(new_bd);
 	if (unlikely(rc))
 		goto error;
 
@@ -268,7 +289,7 @@ void backlight_device_unregister(struct 
 	bd->props = NULL;
 	up(&bd->sem);
 
-	fb_unregister_client(&bd->fb_notif);
+	backlight_unregister_fb(bd);
 
 	class_device_unregister(&bd->class_dev);
 }
diff --git a/drivers/video/backlight/lcd.c b/drivers/video/backlight/lcd.c
index bc8ab00..cab36a4 100644
--- a/drivers/video/backlight/lcd.c
+++ b/drivers/video/backlight/lcd.c
@@ -14,6 +14,51 @@ #include <linux/ctype.h>
 #include <linux/err.h>
 #include <linux/fb.h>
 
+#if defined(CONFIG_FB) || (defined(CONFIG_FB_MODULE) && \
+			   defined(CONFIG_LCD_CLASS_DEVICE_MODULE))
+/* This callback gets called when something important happens inside a
+ * framebuffer driver. We're looking if that important event is blanking,
+ * and if it is, we're switching lcd power as well ...
+ */
+static int fb_notifier_callback(struct notifier_block *self,
+				 unsigned long event, void *data)
+{
+	struct lcd_device *ld;
+	struct fb_event *evdata =(struct fb_event *)data;
+
+	/* If we aren't interested in this event, skip it immediately ... */
+	if (event != FB_EVENT_BLANK)
+		return 0;
+
+	ld = container_of(self, struct lcd_device, fb_notif);
+	down(&ld->sem);
+	if (ld->props)
+		if (!ld->props->check_fb || ld->props->check_fb(evdata->info))
+			ld->props->set_power(ld, *(int *)evdata->data);
+	up(&ld->sem);
+	return 0;
+}
+
+static int lcd_register_fb(struct lcd_device *ld)
+{
+	memset(&ld->fb_notif, 0, sizeof(&ld->fb_notif));
+	ld->fb_notif.notifier_call = fb_notifier_callback;
+	return fb_register_client(&ld->fb_notif);
+}
+
+static void lcd_unregister_fb(struct lcd_device *ld)
+{
+	fb_unregister_client(&ld->fb_notif);
+}
+#else
+static int lcd_register_fb(struct lcd_device *ld)
+{
+	return 0;
+}
+
+#define lcd_unregister_fb(...) do { } while (0)
+#endif /* CONFIG_FB */
+
 static ssize_t lcd_show_power(struct class_device *cdev, char *buf)
 {
 	int rc;
@@ -127,29 +172,6 @@ static struct class_device_attribute lcd
 	DECLARE_ATTR(max_contrast, 0444, lcd_show_max_contrast, NULL),
 };
 
-/* This callback gets called when something important happens inside a
- * framebuffer driver. We're looking if that important event is blanking,
- * and if it is, we're switching lcd power as well ...
- */
-static int fb_notifier_callback(struct notifier_block *self,
-				 unsigned long event, void *data)
-{
-	struct lcd_device *ld;
-	struct fb_event *evdata =(struct fb_event *)data;
-
-	/* If we aren't interested in this event, skip it immediately ... */
-	if (event != FB_EVENT_BLANK)
-		return 0;
-
-	ld = container_of(self, struct lcd_device, fb_notif);
-	down(&ld->sem);
-	if (ld->props)
-		if (!ld->props->check_fb || ld->props->check_fb(evdata->info))
-			ld->props->set_power(ld, *(int *)evdata->data);
-	up(&ld->sem);
-	return 0;
-}
-
 /**
  * lcd_device_register - register a new object of lcd_device class.
  * @name: the name of the new object(must be the same as the name of the
@@ -186,10 +208,8 @@ error:		kfree(new_ld);
 		return ERR_PTR(rc);
 	}
 
-	memset(&new_ld->fb_notif, 0, sizeof(new_ld->fb_notif));
-	new_ld->fb_notif.notifier_call = fb_notifier_callback;
+	rc = lcd_register_fb(new_ld);
 
-	rc = fb_register_client(&new_ld->fb_notif);
 	if (unlikely(rc))
 		goto error;
 
@@ -232,9 +252,7 @@ void lcd_device_unregister(struct lcd_de
 	down(&ld->sem);
 	ld->props = NULL;
 	up(&ld->sem);
-
-	fb_unregister_client(&ld->fb_notif);
-
+	lcd_unregister_fb(ld);
 	class_device_unregister(&ld->class_dev);
 }
 EXPORT_SYMBOL(lcd_device_unregister);


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: Depmod errors on 2.6.17.4/2.6.18-rc1/2.6.18-rc1-mm1
  2006-07-11  3:27       ` Matt Reuther
  2006-07-11  4:52         ` [PATCH] appledisplay/backlight (Depmod errors on 2.6.17.4/2.6.18-rc1/2.6.18-rc1-mm1) Randy.Dunlap
  2006-07-11  5:16         ` [PATCH] sound-miro unknown symbols (Depmod errors on 2.6.17.4/2.6.18-rc1/2.6.18-rc1-mm1) Randy.Dunlap
@ 2006-07-11  6:56         ` Antonino A. Daplas
  2006-07-11 11:20           ` Matt Reuther
  2 siblings, 1 reply; 26+ messages in thread
From: Antonino A. Daplas @ 2006-07-11  6:56 UTC (permalink / raw)
  To: Matt Reuther; +Cc: Andrew Morton, LKML

Matt Reuther wrote:
> On Monday 10 July 2006 11:58 am, Antonino A. Daplas wrote:
>> Matt Reuther wrote:
>>> Quoting "Antonino A. Daplas" <adaplas@gmail.com>:
>>>> CONFIG_FB=n, CONFIG_BACKLIGHT_CLASS_DEVICE=m should not be possible in
>>>> 2.6.18-rc1-mm1 and 2.6.18-rc1.  Can you run kconfig again?
>>>>
>>>> Tony
>>> I am not at the computer right now, but I will try later.
>>>
>>> Here is how I got config-2.6.18-rc1-mm1. I copied this config from
>>> 2.6.18-rc1, which I had created with 'make menuconfig'. I ran 'make
>>> oldconfig' on the config-2.6.18-rc1 and answered the new questions to
>>> generate config-2.6.18-rc1-mm1. I compiled it from there. Does 'make
>>> oldconfig' not work properly anymore?
>> I really don't know.  I have received several bug reports where the
>> main cause was that a kconfig option changed after upgrading kernels.
>>
>> I tested with make menuconfig, and it's not possible to set lcd/backlight
>> options if CONFIG_FB is not set.
>>
>> Tony
> 
> I ran 'make menuconfig' and I got the same warnings. Perhaps CONFIG_FB needs 
> to be part of the 'selects' line for any option that selects the backlight 
> support. I think the USB Apple Cinema display support, which I set as 
> modular, might have selected backlight. I don't need framebuffer support, so 
> I have that shut off. Here are the depmod warnings once again:

Yes, that's the culprit.  I've been thinking for some time to eliminate
the framebuffer dependency from lcd/backlight.  Can you try the patch I sent
in another thread?

Tony

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH] backlight: lcd: Remove dependency from the framebuffer layer
  2006-07-11  4:52         ` [PATCH] appledisplay/backlight (Depmod errors on 2.6.17.4/2.6.18-rc1/2.6.18-rc1-mm1) Randy.Dunlap
  2006-07-11  6:54           ` [PATCH] backlight: lcd: Remove dependency from the framebuffer layer Antonino A. Daplas
@ 2006-07-11  7:04           ` Antonino A. Daplas
  2006-07-11 10:28             ` Andrew Morton
  1 sibling, 1 reply; 26+ messages in thread
From: Antonino A. Daplas @ 2006-07-11  7:04 UTC (permalink / raw)
  To: Randy.Dunlap; +Cc: Matt Reuther, akpm, lkml, Andrew Zabolotny

The backlight and layer should be independent from the framebuffer layer.
It can use the services offered by the framebuffer, but its absence should
not prevent the backlight/lcd layer from functioning.

Signed-off-by: Antonino Daplas <adaplas@pol.net>
---

Oops, the previous patch has extraneous characters. Use this instead.

Tony

 drivers/video/Kconfig               |    2 -
 drivers/video/backlight/Kconfig     |    4 +-
 drivers/video/backlight/backlight.c |   85 ++++++++++++++++++++++-------------
 drivers/video/backlight/lcd.c       |   76 +++++++++++++++++++------------
 4 files changed, 103 insertions(+), 64 deletions(-)

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index c51a54b..9d9d02a 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -1622,7 +1622,7 @@ if FB || SGI_NEWPORT_CONSOLE
 	source "drivers/video/logo/Kconfig"
 endif
 
-if FB && SYSFS
+if SYSFS
 	source "drivers/video/backlight/Kconfig"
 endif
 
diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index 022f9d3..02f1529 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -10,7 +10,7 @@ menuconfig BACKLIGHT_LCD_SUPPORT
 
 config BACKLIGHT_CLASS_DEVICE
         tristate "Lowlevel Backlight controls"
-	depends on BACKLIGHT_LCD_SUPPORT && FB
+	depends on BACKLIGHT_LCD_SUPPORT
 	default m
 	help
 	  This framework adds support for low-level control of the LCD
@@ -26,7 +26,7 @@ config BACKLIGHT_DEVICE
 
 config LCD_CLASS_DEVICE
         tristate "Lowlevel LCD controls"
-	depends on BACKLIGHT_LCD_SUPPORT && FB
+	depends on BACKLIGHT_LCD_SUPPORT
 	default m
 	help
 	  This framework adds support for low-level control of LCD.
diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index 27597c5..6766dfb 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -14,6 +14,57 @@ #include <linux/ctype.h>
 #include <linux/err.h>
 #include <linux/fb.h>
 
+
+#if defined(CONFIG_FB) || (defined(CONFIG_FB_MODULE) && \
+			   defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE))
+/* This callback gets called when something important happens inside a
+ * framebuffer driver. We're looking if that important event is blanking,
+ * and if it is, we're switching backlight power as well ...
+ */
+static int fb_notifier_callback(struct notifier_block *self,
+				unsigned long event, void *data)
+{
+	struct backlight_device *bd;
+	struct fb_event *evdata =(struct fb_event *)data;
+
+	/* If we aren't interested in this event, skip it immediately ... */
+	if (event != FB_EVENT_BLANK)
+		return 0;
+
+	bd = container_of(self, struct backlight_device, fb_notif);
+	down(&bd->sem);
+	if (bd->props)
+		if (!bd->props->check_fb ||
+		    bd->props->check_fb(evdata->info)) {
+			bd->props->fb_blank = *(int *)evdata->data;
+			if (likely(bd->props && bd->props->update_status))
+				bd->props->update_status(bd);
+		}
+	up(&bd->sem);
+	return 0;
+}
+
+static int backlight_register_fb(struct backlight_device *bd)
+{
+	memset(&bd->fb_notif, 0, sizeof(bd->fb_notif));
+	bd->fb_notif.notifier_call = fb_notifier_callback;
+
+	return fb_register_client(&bd->fb_notif);
+}
+
+static void backlight_unregister_fb(struct backlight_device *bd)
+{
+	fb_unregister_client(&bd->fb_notif);
+}
+#else
+static inline int backlight_register_fb(struct backlight_device *bd)
+{
+	return 0;
+}
+
+#define backlight_unregister_fb(...) do { } while (0)
+#endif /* CONFIG_FB */
+
 static ssize_t backlight_show_power(struct class_device *cdev, char *buf)
 {
 	int rc = -ENXIO;
@@ -151,33 +202,6 @@ static struct class_device_attribute bl_
 	DECLARE_ATTR(max_brightness, 0444, backlight_show_max_brightness, NULL),
 };
 
-/* This callback gets called when something important happens inside a
- * framebuffer driver. We're looking if that important event is blanking,
- * and if it is, we're switching backlight power as well ...
- */
-static int fb_notifier_callback(struct notifier_block *self,
-				unsigned long event, void *data)
-{
-	struct backlight_device *bd;
-	struct fb_event *evdata =(struct fb_event *)data;
-
-	/* If we aren't interested in this event, skip it immediately ... */
-	if (event != FB_EVENT_BLANK)
-		return 0;
-
-	bd = container_of(self, struct backlight_device, fb_notif);
-	down(&bd->sem);
-	if (bd->props)
-		if (!bd->props->check_fb ||
-		    bd->props->check_fb(evdata->info)) {
-			bd->props->fb_blank = *(int *)evdata->data;
-			if (likely(bd->props && bd->props->update_status))
-				bd->props->update_status(bd);
-		}
-	up(&bd->sem);
-	return 0;
-}
-
 /**
  * backlight_device_register - create and register a new object of
  *   backlight_device class.
@@ -215,10 +239,7 @@ error:		kfree(new_bd);
 		return ERR_PTR(rc);
 	}
 
-	memset(&new_bd->fb_notif, 0, sizeof(new_bd->fb_notif));
-	new_bd->fb_notif.notifier_call = fb_notifier_callback;
-
-	rc = fb_register_client(&new_bd->fb_notif);
+	rc = backlight_register_fb(new_bd);
 	if (unlikely(rc))
 		goto error;
 
@@ -268,7 +289,7 @@ void backlight_device_unregister(struct 
 	bd->props = NULL;
 	up(&bd->sem);
 
-	fb_unregister_client(&bd->fb_notif);
+	backlight_unregister_fb(bd);
 
 	class_device_unregister(&bd->class_dev);
 }
diff --git a/drivers/video/backlight/lcd.c b/drivers/video/backlight/lcd.c
index bc8ab00..cab36a4 100644
--- a/drivers/video/backlight/lcd.c
+++ b/drivers/video/backlight/lcd.c
@@ -14,6 +14,51 @@ #include <linux/ctype.h>
 #include <linux/err.h>
 #include <linux/fb.h>
 
+#if defined(CONFIG_FB) || (defined(CONFIG_FB_MODULE) && \
+			   defined(CONFIG_LCD_CLASS_DEVICE_MODULE))
+/* This callback gets called when something important happens inside a
+ * framebuffer driver. We're looking if that important event is blanking,
+ * and if it is, we're switching lcd power as well ...
+ */
+static int fb_notifier_callback(struct notifier_block *self,
+				 unsigned long event, void *data)
+{
+	struct lcd_device *ld;
+	struct fb_event *evdata =(struct fb_event *)data;
+
+	/* If we aren't interested in this event, skip it immediately ... */
+	if (event != FB_EVENT_BLANK)
+		return 0;
+
+	ld = container_of(self, struct lcd_device, fb_notif);
+	down(&ld->sem);
+	if (ld->props)
+		if (!ld->props->check_fb || ld->props->check_fb(evdata->info))
+			ld->props->set_power(ld, *(int *)evdata->data);
+	up(&ld->sem);
+	return 0;
+}
+
+static int lcd_register_fb(struct lcd_device *ld)
+{
+	memset(&ld->fb_notif, 0, sizeof(&ld->fb_notif));
+	ld->fb_notif.notifier_call = fb_notifier_callback;
+	return fb_register_client(&ld->fb_notif);
+}
+
+static void lcd_unregister_fb(struct lcd_device *ld)
+{
+	fb_unregister_client(&ld->fb_notif);
+}
+#else
+static int lcd_register_fb(struct lcd_device *ld)
+{
+	return 0;
+}
+
+#define lcd_unregister_fb(...) do { } while (0)
+#endif /* CONFIG_FB */
+
 static ssize_t lcd_show_power(struct class_device *cdev, char *buf)
 {
 	int rc;
@@ -127,29 +172,6 @@ static struct class_device_attribute lcd
 	DECLARE_ATTR(max_contrast, 0444, lcd_show_max_contrast, NULL),
 };
 
-/* This callback gets called when something important happens inside a
- * framebuffer driver. We're looking if that important event is blanking,
- * and if it is, we're switching lcd power as well ...
- */
-static int fb_notifier_callback(struct notifier_block *self,
-				 unsigned long event, void *data)
-{
-	struct lcd_device *ld;
-	struct fb_event *evdata =(struct fb_event *)data;
-
-	/* If we aren't interested in this event, skip it immediately ... */
-	if (event != FB_EVENT_BLANK)
-		return 0;
-
-	ld = container_of(self, struct lcd_device, fb_notif);
-	down(&ld->sem);
-	if (ld->props)
-		if (!ld->props->check_fb || ld->props->check_fb(evdata->info))
-			ld->props->set_power(ld, *(int *)evdata->data);
-	up(&ld->sem);
-	return 0;
-}
-
 /**
  * lcd_device_register - register a new object of lcd_device class.
  * @name: the name of the new object(must be the same as the name of the
@@ -186,10 +208,8 @@ error:		kfree(new_ld);
 		return ERR_PTR(rc);
 	}
 
-	memset(&new_ld->fb_notif, 0, sizeof(new_ld->fb_notif));
-	new_ld->fb_notif.notifier_call = fb_notifier_callback;
+	rc = lcd_register_fb(new_ld);
 
-	rc = fb_register_client(&new_ld->fb_notif);
 	if (unlikely(rc))
 		goto error;
 
@@ -232,9 +252,7 @@ void lcd_device_unregister(struct lcd_de
 	down(&ld->sem);
 	ld->props = NULL;
 	up(&ld->sem);
-
-	fb_unregister_client(&ld->fb_notif);
-
+	lcd_unregister_fb(ld);
 	class_device_unregister(&ld->class_dev);
 }
 EXPORT_SYMBOL(lcd_device_unregister);

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH] backlight: lcd: Remove dependency from the framebuffer layer
  2006-07-11  7:04           ` Antonino A. Daplas
@ 2006-07-11 10:28             ` Andrew Morton
  2006-07-11 10:36               ` Antonino A. Daplas
  2006-07-11 12:45               ` [PATCH] fbdev: Statically link the framebuffer notification functions Antonino A. Daplas
  0 siblings, 2 replies; 26+ messages in thread
From: Andrew Morton @ 2006-07-11 10:28 UTC (permalink / raw)
  To: Antonino A. Daplas; +Cc: rdunlap, mreuther, linux-kernel, zap

On Tue, 11 Jul 2006 15:04:08 +0800
"Antonino A. Daplas" <adaplas@gmail.com> wrote:

> +#if defined(CONFIG_FB) || (defined(CONFIG_FB_MODULE) && \
> +			   defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE))

Peeking at CONFIG_FB_MODULE like this is considered sinful.

<tries to remember why>

Because someone might build a CONFIG_FB=n, CONFIG_FB_MODULE=n kernel, then
later build the fbdev module for that kernel only to find that the
backlight driver doesn't know that the fbdev module is available.

Or something like that.  It's pretty contrived, and I really doubt that
anyone's going to try to cobble together a kernel like that.

Or maybe there's a better reason...

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] backlight: lcd: Remove dependency from the framebuffer layer
  2006-07-11 10:28             ` Andrew Morton
@ 2006-07-11 10:36               ` Antonino A. Daplas
  2006-07-11 12:45               ` [PATCH] fbdev: Statically link the framebuffer notification functions Antonino A. Daplas
  1 sibling, 0 replies; 26+ messages in thread
From: Antonino A. Daplas @ 2006-07-11 10:36 UTC (permalink / raw)
  To: Andrew Morton; +Cc: rdunlap, mreuther, linux-kernel, zap

Andrew Morton wrote:
> On Tue, 11 Jul 2006 15:04:08 +0800
> "Antonino A. Daplas" <adaplas@gmail.com> wrote:
> 
>> +#if defined(CONFIG_FB) || (defined(CONFIG_FB_MODULE) && \
>> +			   defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE))
> 
> Peeking at CONFIG_FB_MODULE like this is considered sinful.
> 
> <tries to remember why>
> 
> Because someone might build a CONFIG_FB=n, CONFIG_FB_MODULE=n kernel, then
> later build the fbdev module for that kernel only to find that the
> backlight driver doesn't know that the fbdev module is available.
> 
> Or something like that.  It's pretty contrived, and I really doubt that
> anyone's going to try to cobble together a kernel like that.
> 
> Or maybe there's a better reason...

That's the simplest method I can think of to prevent impossible
combinations such as CONFIG_FB=m, CONFIG_BACKLIGHT_CLASS_DEVICE=y
without having to use 'depend' or 'select' in Kconfig, or God forbid,
inter_module_get().

I should have added a comment that I also don't like the hackishness
of this method.

Thinking about this a little more carefully now, how about we just statically
link the fb notification?
 
I'll see if I can send an update for that.

Tony

 


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: Depmod errors on 2.6.17.4/2.6.18-rc1/2.6.18-rc1-mm1
  2006-07-11  6:56         ` Depmod errors on 2.6.17.4/2.6.18-rc1/2.6.18-rc1-mm1 Antonino A. Daplas
@ 2006-07-11 11:20           ` Matt Reuther
  2006-07-11 13:12             ` Antonino A. Daplas
  0 siblings, 1 reply; 26+ messages in thread
From: Matt Reuther @ 2006-07-11 11:20 UTC (permalink / raw)
  To: Antonino A. Daplas; +Cc: Andrew Morton, LKML

On Tuesday 11 July 2006 02:56 am, Antonino A. Daplas wrote:
> Matt Reuther wrote:
> > On Monday 10 July 2006 11:58 am, Antonino A. Daplas wrote:
> >> Matt Reuther wrote:
> > I ran 'make menuconfig' and I got the same warnings. Perhaps CONFIG_FB
> > needs to be part of the 'selects' line for any option that selects the
> > backlight support. I think the USB Apple Cinema display support, which I
> > set as modular, might have selected backlight. I don't need framebuffer
> > support, so I have that shut off. Here are the depmod warnings once
> > again:
>
> Yes, that's the culprit.  I've been thinking for some time to eliminate
> the framebuffer dependency from lcd/backlight.  Can you try the patch I
> sent in another thread?
>
> Tony

OK. I will give that a shot, probably this evening.

Shouldn't kconfig recursively figure out dependencies for stuff like this?

I turned on Apple Cinema support without turning on the framebuffer, but 
Cinema needed backlight, which needed framebuffer. I can see kconfig turning 
on backlight, but not checking to see what backlight needed.

There might be similar issues with snd-miro, which seems to want stuff in 
snd_cs4231, which I think I have shut off.

Thanks for your help. I try out your patch later on today, when I can get back 
to my computer.

Matt

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH] fbdev: Statically link the framebuffer notification functions
  2006-07-11 10:28             ` Andrew Morton
  2006-07-11 10:36               ` Antonino A. Daplas
@ 2006-07-11 12:45               ` Antonino A. Daplas
  2006-07-11 12:50                 ` Antonino A. Daplas
  2006-07-11 13:21                 ` Jon Smirl
  1 sibling, 2 replies; 26+ messages in thread
From: Antonino A. Daplas @ 2006-07-11 12:45 UTC (permalink / raw)
  To: Andrew Morton; +Cc: rdunlap, mreuther, linux-kernel, zap

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.

To prevent these, separate out the framebuffer notification mechanism from the
framebuffer layer and permanently link it to the kernel.

Signed-off-by: Antonino Daplas <adaplas@pol.net>
---

Andrew Morton wrote:
> Because someone might build a CONFIG_FB=n, CONFIG_FB_MODULE=n kernel, then
> later build the fbdev module for that kernel only to find that the
> backlight driver doesn't know that the fbdev module is available.
> 
> Or something like that.  It's pretty contrived, and I really doubt that
> anyone's going to try to cobble together a kernel like that.
> 
> Or maybe there's a better reason...
> 

Andrew,

Here's the patch as promised. The changelog should say it all. Statically 
linking the notification mechanism is not that expensive as the code is very
small. Also, future subsystems that requires fb notification need not play
dependency games.

I prefer this over the previous patch. If you do apply this, you have to
revert backlight-lcd-remove-dependency-from-the-framebuffer-layer.patch.
This patch leaves the code in the backlight directory untouched.

Tony

 drivers/video/Makefile          |    1 +
 drivers/video/backlight/Kconfig |    4 +--
 drivers/video/fb_notify.c       |   46 +++++++++++++++++++++++++++++++++
 drivers/video/fbmem.c           |   54 ++++++++-------------------------------
 include/linux/fb.h              |    2 +
 5 files changed, 61 insertions(+), 46 deletions(-)

diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index 6283d01..be7f1c9 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -4,6 +4,7 @@ # Rewritten to use lists instead of if-s
 
 # Each configuration option enables a list of files.
 
+obj-y                             += fb_notify.o
 obj-$(CONFIG_FB)                  += fb.o
 fb-y                              := fbmem.o fbmon.o fbcmap.o fbsysfs.o \
                                      modedb.o fbcvt.o
diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index 022f9d3..02f1529 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -10,7 +10,7 @@ menuconfig BACKLIGHT_LCD_SUPPORT
 
 config BACKLIGHT_CLASS_DEVICE
         tristate "Lowlevel Backlight controls"
-	depends on BACKLIGHT_LCD_SUPPORT && FB
+	depends on BACKLIGHT_LCD_SUPPORT
 	default m
 	help
 	  This framework adds support for low-level control of the LCD
@@ -26,7 +26,7 @@ config BACKLIGHT_DEVICE
 
 config LCD_CLASS_DEVICE
         tristate "Lowlevel LCD controls"
-	depends on BACKLIGHT_LCD_SUPPORT && FB
+	depends on BACKLIGHT_LCD_SUPPORT
 	default m
 	help
 	  This framework adds support for low-level control of LCD.
diff --git a/drivers/video/fb_notify.c b/drivers/video/fb_notify.c
new file mode 100644
index 0000000..8c02038
--- /dev/null
+++ b/drivers/video/fb_notify.c
@@ -0,0 +1,46 @@
+/*
+ *  linux/drivers/video/fb_notify.c
+ *
+ *  Copyright (C) 2006 Antonino Daplas <adaplas@pol.net>
+ *
+ *	2001 - Documented with DocBook
+ *	- Brad Douglas <brad@neruo.com>
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file COPYING in the main directory of this archive
+ * for more details.
+ */
+#include <linux/fb.h>
+#include <linux/notifier.h>
+
+static BLOCKING_NOTIFIER_HEAD(fb_notifier_list);
+
+/**
+ *	fb_register_client - register a client notifier
+ *	@nb: notifier block to callback on events
+ */
+int fb_register_client(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_register(&fb_notifier_list, nb);
+}
+EXPORT_SYMBOL(fb_register_client);
+
+/**
+ *	fb_unregister_client - unregister a client notifier
+ *	@nb: notifier block to callback on events
+ */
+int fb_unregister_client(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_unregister(&fb_notifier_list, nb);
+}
+EXPORT_SYMBOL(fb_unregister_client);
+
+/**
+ * fb_notifier_call_chain - notify clients of fb_events
+ *
+ */
+int fb_notifier_call_chain(unsigned long val, void *v)
+{
+	return blocking_notifier_call_chain(&fb_notifier_list, val, v);
+}
+EXPORT_SYMBOL_GPL(fb_notifier_call_chain);
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index 4fc9df4..17961e3 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -52,7 +52,6 @@ #include <linux/fb.h>
 
 #define FBPIXMAPSIZE	(1024 * 8)
 
-static BLOCKING_NOTIFIER_HEAD(fb_notifier_list);
 struct fb_info *registered_fb[FB_MAX];
 int num_registered_fb;
 
@@ -791,8 +790,7 @@ fb_set_var(struct fb_info *info, struct 
 
 		    event.info = info;
 		    event.data = &mode1;
-		    ret = blocking_notifier_call_chain(&fb_notifier_list,
-					      FB_EVENT_MODE_DELETE, &event);
+		    ret = fb_notifier_call_chain(FB_EVENT_MODE_DELETE, &event);
 		}
 
 		if (!ret)
@@ -837,8 +835,7 @@ fb_set_var(struct fb_info *info, struct 
 
 				info->flags &= ~FBINFO_MISC_USEREVENT;
 				event.info = info;
-				blocking_notifier_call_chain(&fb_notifier_list,
-						evnt, &event);
+				fb_notifier_call_chain(evnt, &event);
 			}
 		}
 	}
@@ -861,8 +858,7 @@ fb_blank(struct fb_info *info, int blank
 
 		event.info = info;
 		event.data = &blank;
-		blocking_notifier_call_chain(&fb_notifier_list,
-				FB_EVENT_BLANK, &event);
+		fb_notifier_call_chain(FB_EVENT_BLANK, &event);
 	}
 
  	return ret;
@@ -933,8 +929,7 @@ fb_ioctl(struct inode *inode, struct fil
 		con2fb.framebuffer = -1;
 		event.info = info;
 		event.data = &con2fb;
-		blocking_notifier_call_chain(&fb_notifier_list,
-				    FB_EVENT_GET_CONSOLE_MAP, &event);
+		fb_notifier_call_chain(FB_EVENT_GET_CONSOLE_MAP, &event);
 		return copy_to_user(argp, &con2fb,
 				    sizeof(con2fb)) ? -EFAULT : 0;
 	case FBIOPUT_CON2FBMAP:
@@ -952,9 +947,8 @@ #endif /* CONFIG_KMOD */
 		    return -EINVAL;
 		event.info = info;
 		event.data = &con2fb;
-		return blocking_notifier_call_chain(&fb_notifier_list,
-					   FB_EVENT_SET_CONSOLE_MAP,
-					   &event);
+		return fb_notifier_call_chain(FB_EVENT_SET_CONSOLE_MAP,
+					      &event);
 	case FBIOBLANK:
 		acquire_console_sem();
 		info->flags |= FBINFO_MISC_USEREVENT;
@@ -1330,8 +1324,7 @@ register_framebuffer(struct fb_info *fb_
 	registered_fb[i] = fb_info;
 
 	event.info = fb_info;
-	blocking_notifier_call_chain(&fb_notifier_list,
-			    FB_EVENT_FB_REGISTERED, &event);
+	fb_notifier_call_chain(FB_EVENT_FB_REGISTERED, &event);
 	return 0;
 }
 
@@ -1365,30 +1358,11 @@ unregister_framebuffer(struct fb_info *f
 	fb_cleanup_class_device(fb_info);
 	class_device_destroy(fb_class, MKDEV(FB_MAJOR, i));
 	event.info = fb_info;
-	blocking_notifier_call_chain(&fb_notifier_list,
-				     FB_EVENT_FB_UNREGISTERED, &event);
+	fb_notifier_call_chain(FB_EVENT_FB_UNREGISTERED, &event);
 	return 0;
 }
 
 /**
- *	fb_register_client - register a client notifier
- *	@nb: notifier block to callback on events
- */
-int fb_register_client(struct notifier_block *nb)
-{
-	return blocking_notifier_chain_register(&fb_notifier_list, nb);
-}
-
-/**
- *	fb_unregister_client - unregister a client notifier
- *	@nb: notifier block to callback on events
- */
-int fb_unregister_client(struct notifier_block *nb)
-{
-	return blocking_notifier_chain_unregister(&fb_notifier_list, nb);
-}
-
-/**
  *	fb_set_suspend - low level driver signals suspend
  *	@info: framebuffer affected
  *	@state: 0 = resuming, !=0 = suspending
@@ -1403,13 +1377,11 @@ void fb_set_suspend(struct fb_info *info
 
 	event.info = info;
 	if (state) {
-		blocking_notifier_call_chain(&fb_notifier_list,
-				FB_EVENT_SUSPEND, &event);
+		fb_notifier_call_chain(FB_EVENT_SUSPEND, &event);
 		info->state = FBINFO_STATE_SUSPENDED;
 	} else {
 		info->state = FBINFO_STATE_RUNNING;
-		blocking_notifier_call_chain(&fb_notifier_list,
-				FB_EVENT_RESUME, &event);
+		fb_notifier_call_chain(FB_EVENT_RESUME, &event);
 	}
 }
 
@@ -1480,9 +1452,7 @@ int fb_new_modelist(struct fb_info *info
 
 	if (!list_empty(&info->modelist)) {
 		event.info = info;
-		err = blocking_notifier_call_chain(&fb_notifier_list,
-					   FB_EVENT_NEW_MODELIST,
-					   &event);
+		err = fb_notifier_call_chain(FB_EVENT_NEW_MODELIST, &event);
 	}
 
 	return err;
@@ -1594,8 +1564,6 @@ EXPORT_SYMBOL(fb_blank);
 EXPORT_SYMBOL(fb_pan_display);
 EXPORT_SYMBOL(fb_get_buffer_offset);
 EXPORT_SYMBOL(fb_set_suspend);
-EXPORT_SYMBOL(fb_register_client);
-EXPORT_SYMBOL(fb_unregister_client);
 EXPORT_SYMBOL(fb_get_options);
 
 MODULE_LICENSE("GPL");
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 405f44e..4ad0673 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -524,7 +524,7 @@ struct fb_event {
 
 extern int fb_register_client(struct notifier_block *nb);
 extern int fb_unregister_client(struct notifier_block *nb);
-
+extern int fb_notifier_call_chain(unsigned long val, void *v);
 /*
  * Pixmap structure definition
  *

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH] fbdev: Statically link the framebuffer notification functions
  2006-07-11 12:45               ` [PATCH] fbdev: Statically link the framebuffer notification functions Antonino A. Daplas
@ 2006-07-11 12:50                 ` Antonino A. Daplas
  2006-07-11 13:21                 ` Jon Smirl
  1 sibling, 0 replies; 26+ messages in thread
From: Antonino A. Daplas @ 2006-07-11 12:50 UTC (permalink / raw)
  To: Antonino A. Daplas; +Cc: Andrew Morton, rdunlap, mreuther, linux-kernel, zap

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.

To prevent dependency problems, separate out the framebuffer
notification mechanism from the framebuffer layer and permanently link it
to the kernel.

Signed-off-by: Antonino Daplas <adaplas@pol.net>
---

Another oops, forgot to remove the if FB in Kconfig.

Tony

 drivers/video/Kconfig           |    2 +
 drivers/video/Makefile          |    1 +
 drivers/video/backlight/Kconfig |    4 +--
 drivers/video/fb_notify.c       |   46 +++++++++++++++++++++++++++++++++
 drivers/video/fbmem.c           |   54 ++++++++-------------------------------
 include/linux/fb.h              |    2 +
 6 files changed, 62 insertions(+), 47 deletions(-)

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index c51a54b..9d9d02a 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -1622,7 +1622,7 @@ if FB || SGI_NEWPORT_CONSOLE
 	source "drivers/video/logo/Kconfig"
 endif
 
-if FB && SYSFS
+if SYSFS
 	source "drivers/video/backlight/Kconfig"
 endif
 
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index 6283d01..be7f1c9 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -4,6 +4,7 @@ # Rewritten to use lists instead of if-s
 
 # Each configuration option enables a list of files.
 
+obj-y                             += fb_notify.o
 obj-$(CONFIG_FB)                  += fb.o
 fb-y                              := fbmem.o fbmon.o fbcmap.o fbsysfs.o \
                                      modedb.o fbcvt.o
diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index 022f9d3..02f1529 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -10,7 +10,7 @@ menuconfig BACKLIGHT_LCD_SUPPORT
 
 config BACKLIGHT_CLASS_DEVICE
         tristate "Lowlevel Backlight controls"
-	depends on BACKLIGHT_LCD_SUPPORT && FB
+	depends on BACKLIGHT_LCD_SUPPORT
 	default m
 	help
 	  This framework adds support for low-level control of the LCD
@@ -26,7 +26,7 @@ config BACKLIGHT_DEVICE
 
 config LCD_CLASS_DEVICE
         tristate "Lowlevel LCD controls"
-	depends on BACKLIGHT_LCD_SUPPORT && FB
+	depends on BACKLIGHT_LCD_SUPPORT
 	default m
 	help
 	  This framework adds support for low-level control of LCD.
diff --git a/drivers/video/fb_notify.c b/drivers/video/fb_notify.c
new file mode 100644
index 0000000..8c02038
--- /dev/null
+++ b/drivers/video/fb_notify.c
@@ -0,0 +1,46 @@
+/*
+ *  linux/drivers/video/fb_notify.c
+ *
+ *  Copyright (C) 2006 Antonino Daplas <adaplas@pol.net>
+ *
+ *	2001 - Documented with DocBook
+ *	- Brad Douglas <brad@neruo.com>
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file COPYING in the main directory of this archive
+ * for more details.
+ */
+#include <linux/fb.h>
+#include <linux/notifier.h>
+
+static BLOCKING_NOTIFIER_HEAD(fb_notifier_list);
+
+/**
+ *	fb_register_client - register a client notifier
+ *	@nb: notifier block to callback on events
+ */
+int fb_register_client(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_register(&fb_notifier_list, nb);
+}
+EXPORT_SYMBOL(fb_register_client);
+
+/**
+ *	fb_unregister_client - unregister a client notifier
+ *	@nb: notifier block to callback on events
+ */
+int fb_unregister_client(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_unregister(&fb_notifier_list, nb);
+}
+EXPORT_SYMBOL(fb_unregister_client);
+
+/**
+ * fb_notifier_call_chain - notify clients of fb_events
+ *
+ */
+int fb_notifier_call_chain(unsigned long val, void *v)
+{
+	return blocking_notifier_call_chain(&fb_notifier_list, val, v);
+}
+EXPORT_SYMBOL_GPL(fb_notifier_call_chain);
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index 4fc9df4..17961e3 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -52,7 +52,6 @@ #include <linux/fb.h>
 
 #define FBPIXMAPSIZE	(1024 * 8)
 
-static BLOCKING_NOTIFIER_HEAD(fb_notifier_list);
 struct fb_info *registered_fb[FB_MAX];
 int num_registered_fb;
 
@@ -791,8 +790,7 @@ fb_set_var(struct fb_info *info, struct 
 
 		    event.info = info;
 		    event.data = &mode1;
-		    ret = blocking_notifier_call_chain(&fb_notifier_list,
-					      FB_EVENT_MODE_DELETE, &event);
+		    ret = fb_notifier_call_chain(FB_EVENT_MODE_DELETE, &event);
 		}
 
 		if (!ret)
@@ -837,8 +835,7 @@ fb_set_var(struct fb_info *info, struct 
 
 				info->flags &= ~FBINFO_MISC_USEREVENT;
 				event.info = info;
-				blocking_notifier_call_chain(&fb_notifier_list,
-						evnt, &event);
+				fb_notifier_call_chain(evnt, &event);
 			}
 		}
 	}
@@ -861,8 +858,7 @@ fb_blank(struct fb_info *info, int blank
 
 		event.info = info;
 		event.data = &blank;
-		blocking_notifier_call_chain(&fb_notifier_list,
-				FB_EVENT_BLANK, &event);
+		fb_notifier_call_chain(FB_EVENT_BLANK, &event);
 	}
 
  	return ret;
@@ -933,8 +929,7 @@ fb_ioctl(struct inode *inode, struct fil
 		con2fb.framebuffer = -1;
 		event.info = info;
 		event.data = &con2fb;
-		blocking_notifier_call_chain(&fb_notifier_list,
-				    FB_EVENT_GET_CONSOLE_MAP, &event);
+		fb_notifier_call_chain(FB_EVENT_GET_CONSOLE_MAP, &event);
 		return copy_to_user(argp, &con2fb,
 				    sizeof(con2fb)) ? -EFAULT : 0;
 	case FBIOPUT_CON2FBMAP:
@@ -952,9 +947,8 @@ #endif /* CONFIG_KMOD */
 		    return -EINVAL;
 		event.info = info;
 		event.data = &con2fb;
-		return blocking_notifier_call_chain(&fb_notifier_list,
-					   FB_EVENT_SET_CONSOLE_MAP,
-					   &event);
+		return fb_notifier_call_chain(FB_EVENT_SET_CONSOLE_MAP,
+					      &event);
 	case FBIOBLANK:
 		acquire_console_sem();
 		info->flags |= FBINFO_MISC_USEREVENT;
@@ -1330,8 +1324,7 @@ register_framebuffer(struct fb_info *fb_
 	registered_fb[i] = fb_info;
 
 	event.info = fb_info;
-	blocking_notifier_call_chain(&fb_notifier_list,
-			    FB_EVENT_FB_REGISTERED, &event);
+	fb_notifier_call_chain(FB_EVENT_FB_REGISTERED, &event);
 	return 0;
 }
 
@@ -1365,30 +1358,11 @@ unregister_framebuffer(struct fb_info *f
 	fb_cleanup_class_device(fb_info);
 	class_device_destroy(fb_class, MKDEV(FB_MAJOR, i));
 	event.info = fb_info;
-	blocking_notifier_call_chain(&fb_notifier_list,
-				     FB_EVENT_FB_UNREGISTERED, &event);
+	fb_notifier_call_chain(FB_EVENT_FB_UNREGISTERED, &event);
 	return 0;
 }
 
 /**
- *	fb_register_client - register a client notifier
- *	@nb: notifier block to callback on events
- */
-int fb_register_client(struct notifier_block *nb)
-{
-	return blocking_notifier_chain_register(&fb_notifier_list, nb);
-}
-
-/**
- *	fb_unregister_client - unregister a client notifier
- *	@nb: notifier block to callback on events
- */
-int fb_unregister_client(struct notifier_block *nb)
-{
-	return blocking_notifier_chain_unregister(&fb_notifier_list, nb);
-}
-
-/**
  *	fb_set_suspend - low level driver signals suspend
  *	@info: framebuffer affected
  *	@state: 0 = resuming, !=0 = suspending
@@ -1403,13 +1377,11 @@ void fb_set_suspend(struct fb_info *info
 
 	event.info = info;
 	if (state) {
-		blocking_notifier_call_chain(&fb_notifier_list,
-				FB_EVENT_SUSPEND, &event);
+		fb_notifier_call_chain(FB_EVENT_SUSPEND, &event);
 		info->state = FBINFO_STATE_SUSPENDED;
 	} else {
 		info->state = FBINFO_STATE_RUNNING;
-		blocking_notifier_call_chain(&fb_notifier_list,
-				FB_EVENT_RESUME, &event);
+		fb_notifier_call_chain(FB_EVENT_RESUME, &event);
 	}
 }
 
@@ -1480,9 +1452,7 @@ int fb_new_modelist(struct fb_info *info
 
 	if (!list_empty(&info->modelist)) {
 		event.info = info;
-		err = blocking_notifier_call_chain(&fb_notifier_list,
-					   FB_EVENT_NEW_MODELIST,
-					   &event);
+		err = fb_notifier_call_chain(FB_EVENT_NEW_MODELIST, &event);
 	}
 
 	return err;
@@ -1594,8 +1564,6 @@ EXPORT_SYMBOL(fb_blank);
 EXPORT_SYMBOL(fb_pan_display);
 EXPORT_SYMBOL(fb_get_buffer_offset);
 EXPORT_SYMBOL(fb_set_suspend);
-EXPORT_SYMBOL(fb_register_client);
-EXPORT_SYMBOL(fb_unregister_client);
 EXPORT_SYMBOL(fb_get_options);
 
 MODULE_LICENSE("GPL");
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 405f44e..4ad0673 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -524,7 +524,7 @@ struct fb_event {
 
 extern int fb_register_client(struct notifier_block *nb);
 extern int fb_unregister_client(struct notifier_block *nb);
-
+extern int fb_notifier_call_chain(unsigned long val, void *v);
 /*
  * Pixmap structure definition
  *

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: Depmod errors on 2.6.17.4/2.6.18-rc1/2.6.18-rc1-mm1
  2006-07-11 11:20           ` Matt Reuther
@ 2006-07-11 13:12             ` Antonino A. Daplas
  2006-07-12 12:41               ` Matt Reuther
  0 siblings, 1 reply; 26+ messages in thread
From: Antonino A. Daplas @ 2006-07-11 13:12 UTC (permalink / raw)
  To: Matt Reuther; +Cc: Andrew Morton, LKML

Matt Reuther wrote:
> On Tuesday 11 July 2006 02:56 am, Antonino A. Daplas wrote:
>> Matt Reuther wrote:
>>> On Monday 10 July 2006 11:58 am, Antonino A. Daplas wrote:
>>>> Matt Reuther wrote:
>>> I ran 'make menuconfig' and I got the same warnings. Perhaps CONFIG_FB
>>> needs to be part of the 'selects' line for any option that selects the
>>> backlight support. I think the USB Apple Cinema display support, which I
>>> set as modular, might have selected backlight. I don't need framebuffer
>>> support, so I have that shut off. Here are the depmod warnings once
>>> again:
>> Yes, that's the culprit.  I've been thinking for some time to eliminate
>> the framebuffer dependency from lcd/backlight.  Can you try the patch I
>> sent in another thread?
>>
>> Tony
> 
> OK. I will give that a shot, probably this evening.
> 
> Shouldn't kconfig recursively figure out dependencies for stuff like this?

'select' doesn't. So does 'depends on'. Anyway, I consider it wasteful to
also compile in fb when backlight/lcd is the only one needed. So try instead
the other patch (Statically link the framebuffer notification functions).

> 
> I turned on Apple Cinema support without turning on the framebuffer, but 
> Cinema needed backlight, which needed framebuffer. I can see kconfig turning 
> on backlight, but not checking to see what backlight needed.

Yes, known problem with select.

> 
> There might be similar issues with snd-miro, which seems to want stuff in 
> snd_cs4231, which I think I have shut off.
> 
> Thanks for your help. I try out your patch later on today, when I can get back 
> to my computer.
> 

Thanks for the report too.

Tony

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] fbdev: Statically link the framebuffer notification functions
  2006-07-11 12:45               ` [PATCH] fbdev: Statically link the framebuffer notification functions Antonino A. Daplas
  2006-07-11 12:50                 ` Antonino A. Daplas
@ 2006-07-11 13:21                 ` Jon Smirl
  2006-07-11 13:40                   ` Antonino A. Daplas
  1 sibling, 1 reply; 26+ messages in thread
From: Jon Smirl @ 2006-07-11 13:21 UTC (permalink / raw)
  To: Antonino A. Daplas; +Cc: Andrew Morton, rdunlap, mreuther, linux-kernel, zap

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.

Another solution would be to put this code into a tiny module and then
have everyone depend on it.

-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] fbdev: Statically link the framebuffer notification functions
  2006-07-11 13:21                 ` Jon Smirl
@ 2006-07-11 13:40                   ` Antonino A. Daplas
  2006-07-11 13:46                     ` Jon Smirl
  0 siblings, 1 reply; 26+ messages in thread
From: Antonino A. Daplas @ 2006-07-11 13:40 UTC (permalink / raw)
  To: Jon Smirl; +Cc: Andrew Morton, rdunlap, mreuther, linux-kernel, zap

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.

Tony



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] fbdev: Statically link the framebuffer notification functions
  2006-07-11 13:40                   ` Antonino A. Daplas
@ 2006-07-11 13:46                     ` Jon Smirl
  2006-07-11 14:34                       ` Antonino A. Daplas
  0 siblings, 1 reply; 26+ messages in thread
From: Jon Smirl @ 2006-07-11 13:46 UTC (permalink / raw)
  To: Antonino A. Daplas; +Cc: Andrew Morton, rdunlap, mreuther, linux-kernel, zap

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. The symbol is
being passed implicitly via the entry point names.

Just wrap fb_notifier in a module and it will get tracked.

>
> Tony
>
>
>


-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] fbdev: Statically link the framebuffer notification functions
  2006-07-11 13:46                     ` Jon Smirl
@ 2006-07-11 14:34                       ` Antonino A. Daplas
  2006-07-11 14:43                         ` Jon Smirl
  0 siblings, 1 reply; 26+ messages in thread
From: Antonino A. Daplas @ 2006-07-11 14:34 UTC (permalink / raw)
  To: Jon Smirl; +Cc: Andrew Morton, rdunlap, mreuther, linux-kernel, zap

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 symbol is
> being passed implicitly via the entry point names.
> 
> Just wrap fb_notifier in a module and it will get tracked.

No, there's no need to do this. It just adds more complexity to
the already complicated dependency.

Tony

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] fbdev: Statically link the framebuffer notification functions
  2006-07-11 14:34                       ` Antonino A. Daplas
@ 2006-07-11 14:43                         ` Jon Smirl
  2006-07-11 14:50                           ` Antonino A. Daplas
  0 siblings, 1 reply; 26+ messages in thread
From: Jon Smirl @ 2006-07-11 14:43 UTC (permalink / raw)
  To: Antonino A. Daplas; +Cc: Andrew Morton, rdunlap, mreuther, linux-kernel, zap

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 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?

-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] fbdev: Statically link the framebuffer notification functions
  2006-07-11 14:43                         ` Jon Smirl
@ 2006-07-11 14:50                           ` Antonino A. Daplas
  2006-07-11 15:03                             ` Jon Smirl
  0 siblings, 1 reply; 26+ messages in thread
From: Antonino A. Daplas @ 2006-07-11 14:50 UTC (permalink / raw)
  To: Jon Smirl; +Cc: Andrew Morton, rdunlap, mreuther, linux-kernel, zap

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


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] fbdev: Statically link the framebuffer notification functions
  2006-07-11 14:50                           ` Antonino A. Daplas
@ 2006-07-11 15:03                             ` Jon Smirl
  2006-07-11 15:21                               ` Antonino A. Daplas
  0 siblings, 1 reply; 26+ messages in thread
From: Jon Smirl @ 2006-07-11 15:03 UTC (permalink / raw)
  To: Antonino A. Daplas; +Cc: Andrew Morton, rdunlap, mreuther, linux-kernel, zap

On 7/11/06, Antonino A. Daplas <adaplas@gmail.com> wrote:
> > 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?

Dangling references during the load/unload process. That was
inter_module's problem.

>
> > 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.

The generic code could create notifier chains with a name. The modules
would then use the name to attach. Now you don't need the fb_notifier
code.

-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] fbdev: Statically link the framebuffer notification functions
  2006-07-11 15:03                             ` Jon Smirl
@ 2006-07-11 15:21                               ` Antonino A. Daplas
  0 siblings, 0 replies; 26+ messages in thread
From: Antonino A. Daplas @ 2006-07-11 15:21 UTC (permalink / raw)
  To: Jon Smirl; +Cc: Andrew Morton, rdunlap, mreuther, linux-kernel, zap

Jon Smirl wrote:
> On 7/11/06, Antonino A. Daplas <adaplas@gmail.com> wrote:
>> > 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?
> 
> Dangling references during the load/unload process. That was
> inter_module's problem.

That won't happen. If fbdev unloads, then the module that does the
notification disappears, and the clients won't receive notifications.
If the client is the one that unloads first, it unregisters its notifier
block, and that's one less client for fbdev to notify (And fbdev doesn't
know how many clients are there, that's internal to the notifier). And
since registration, unregistration and the call to the callout function
in the notifier block are protected by a semaphore (in the blocking type),
the danger of unregistration while in the midst of a notification is
removed.

Tony

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: Depmod errors on 2.6.17.4/2.6.18-rc1/2.6.18-rc1-mm1
  2006-07-11 13:12             ` Antonino A. Daplas
@ 2006-07-12 12:41               ` Matt Reuther
  2006-07-12 15:25                 ` Randy.Dunlap
  0 siblings, 1 reply; 26+ messages in thread
From: Matt Reuther @ 2006-07-12 12:41 UTC (permalink / raw)
  To: Antonino A. Daplas; +Cc: Andrew Morton, LKML

The depmod problems are gone, except for the reiser4 warning about 
generic_file_read. I thought I had the right patch for that, too, but I guess 
not.

Thanks for all the help!
Matt

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: Depmod errors on 2.6.17.4/2.6.18-rc1/2.6.18-rc1-mm1
  2006-07-12 12:41               ` Matt Reuther
@ 2006-07-12 15:25                 ` Randy.Dunlap
  0 siblings, 0 replies; 26+ messages in thread
From: Randy.Dunlap @ 2006-07-12 15:25 UTC (permalink / raw)
  To: Matt Reuther; +Cc: adaplas, akpm, linux-kernel

On Wed, 12 Jul 2006 08:41:48 -0400 Matt Reuther wrote:

> The depmod problems are gone, except for the reiser4 warning about 
> generic_file_read. I thought I had the right patch for that, too, but I guess 
> not.

This one seems to be the correct one:
http://marc.theaimsgroup.com/?l=linux-kernel&m=115253892505274&w=2

---
~Randy

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2006-07-12 15:22 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-07-10 12:32 Depmod errors on 2.6.17.4/2.6.18-rc1/2.6.18-rc1-mm1 Matt Reuther
2006-07-10 13:02 ` Antonino A. Daplas
2006-07-10 15:32   ` Matt Reuther
2006-07-10 15:58     ` Antonino A. Daplas
2006-07-11  3:27       ` Matt Reuther
2006-07-11  4:52         ` [PATCH] appledisplay/backlight (Depmod errors on 2.6.17.4/2.6.18-rc1/2.6.18-rc1-mm1) Randy.Dunlap
2006-07-11  6:54           ` [PATCH] backlight: lcd: Remove dependency from the framebuffer layer Antonino A. Daplas
2006-07-11  7:04           ` Antonino A. Daplas
2006-07-11 10:28             ` Andrew Morton
2006-07-11 10:36               ` Antonino A. Daplas
2006-07-11 12:45               ` [PATCH] fbdev: Statically link the framebuffer notification functions Antonino A. Daplas
2006-07-11 12:50                 ` Antonino A. Daplas
2006-07-11 13:21                 ` Jon Smirl
2006-07-11 13:40                   ` Antonino A. Daplas
2006-07-11 13:46                     ` Jon Smirl
2006-07-11 14:34                       ` Antonino A. Daplas
2006-07-11 14:43                         ` Jon Smirl
2006-07-11 14:50                           ` Antonino A. Daplas
2006-07-11 15:03                             ` Jon Smirl
2006-07-11 15:21                               ` Antonino A. Daplas
2006-07-11  5:16         ` [PATCH] sound-miro unknown symbols (Depmod errors on 2.6.17.4/2.6.18-rc1/2.6.18-rc1-mm1) Randy.Dunlap
2006-07-11  6:56         ` Depmod errors on 2.6.17.4/2.6.18-rc1/2.6.18-rc1-mm1 Antonino A. Daplas
2006-07-11 11:20           ` Matt Reuther
2006-07-11 13:12             ` Antonino A. Daplas
2006-07-12 12:41               ` Matt Reuther
2006-07-12 15:25                 ` Randy.Dunlap

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox