Prev: mtd: Extend physmap_of to let the device tree specify the parition probe
Next: watchdog docs: Fix use of WDIOC_SETOPTIONS ioctl.
From: Bruno Prémont on 7 Apr 2010 16:30 On Wed, 07 April 2010 Randy Dunlap <randy.dunlap(a)oracle.com> wrote: > On Wed, 7 Apr 2010 20:44:04 +0200 Bruno Prémont wrote: > > > On Tue, 06 April 2010 Randy Dunlap <randy.dunlap(a)oracle.com> wrote: > > > > > > > > One of my attempts did end up with a circular loop with regard to FB > > > > (some of the FB drivers did select INPUT)? > > > > > > (not that I can find) > > > > > > CONFIG_VT does select INPUT > > > and CONFIG_DRM_I915 does > > > select INPUT if ACPI > > > > A newer attempt still produces the same result: > > > > drivers/input/Kconfig:9:error: found recursive dependency: INPUT -> > > HID_SUPPORT -> HID_PICOLCD_FB -> FB -> FB_STI -> VT -> INPUT > > > > (it's only FB which causes the loop, LEDS, LCD and BACKLIGHT are fine) > > (yes, so I see) > > > This is with following patch on top of the improved deps patch I sent > > a few minutes ago deeper in this thread. > > > > Is there a way around this? > > > > Well, lesson #1 is that select is evil^W^W should only be used to enable > library-like code, or as Documentation/kbuild/kconfig-language.txt says: > > Note: > select should be used with care. select will force > a symbol to a value without visiting the dependencies. > By abusing select you are able to select a symbol FOO even > if FOO depends on BAR that is not set. > In general use select only for non-visible symbols > (no prompts anywhere) and for symbols with no dependencies. > That will limit the usefulness but on the other hand avoid > the illegal configurations all over. > kconfig should one day warn about such things. I've read that paragraph and I would definitely prefer not to have to use select for what this patch should achieve! It should really be a helper for the user going through menuconfig like "please check all what is needed to satisfy this item's dependencies" A "try-select-deep" or whatever it could be called. A well-behaved "GUI" implementation of this would show what would get newly checked and give the user the opportunity to not proceed or fine-tune y/m choices. > (more below) > > > > > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig > > index 782a34e..711c091 100644 > > --- a/drivers/hid/Kconfig > > +++ b/drivers/hid/Kconfig > > @@ -285,7 +285,7 @@ config HID_PICOLCD_FB > > bool "Framebuffer support" > > default !EMBEDDED > > depends on HID_PICOLCD > > - depends on HID_PICOLCD=FB || FB=y > > + select FB > > If you'll go back to the unpatched (by this patch) version here, > it seems to work OK. So best is probably to just forget about all of this non-leaf select usage for now. (and not have one half doing one way and the other half doing it the other way) Thanks, Bruno -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Jiri Kosina on 8 Apr 2010 08:50 On Wed, 7 Apr 2010, Bruno Prémont wrote: > HID_PICOLCD should depend on LCD_CLASS_DEVICE, otherwise the > build fails when HID_PICOLCD=y and LCD_CLASS_DEVICE=m: > > hid-picolcd.c:(.text+0x84523f): undefined reference to `lcd_device_unregister' > hid-picolcd.c:(.text+0x8478ab): undefined reference to `lcd_device_register' > hid-picolcd.c:(.text+0x84c15f): undefined reference to `lcd_device_unregister' > > Same applies to FB, BACKLIGHT_CLASS_DEVICE and LEDS_CLASS. > > Add suboptions for those features to handle the deps on kbuild side > and just check HID_PICOLCD_* in the code. > > Reported-by: Randy Dunlap <randy.dunlap(a)oracle.com> > Signed-off-by: Bruno Prémont <bonbons(a)linux-vserver.org> > --- > > On Wed, 07 April 2010 Randy Dunlap <randy.dunlap(a)oracle.com> wrote: > > All of these user-visible kconfig options need help text also... > > > > Here is a better patch, with added documentation and stripped > select clauses under HID_PICOLCD as they are handled by HID_PICOLCD_FB. > > > > > drivers/hid/Kconfig | 53 +++++++++++++++++++++++++++++++++++++------- > drivers/hid/hid-picolcd.c | 40 +++++++++++++++++----------------- > 2 files changed, 64 insertions(+), 29 deletions(-) > > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig > index a2ecd83..782a34e 100644 > --- a/drivers/hid/Kconfig > +++ b/drivers/hid/Kconfig > @@ -265,11 +265,6 @@ config HID_PETALYNX > config HID_PICOLCD > tristate "PicoLCD (graphic version)" > depends on USB_HID > - select FB_DEFERRED_IO if FB > - select FB_SYS_FILLRECT if FB > - select FB_SYS_COPYAREA if FB > - select FB_SYS_IMAGEBLIT if FB > - select FB_SYS_FOPS if FB > ---help--- > This provides support for Minibox PicoLCD devices, currently > only the graphical ones are supported. > @@ -277,14 +272,54 @@ config HID_PICOLCD > This includes support for the following device features: > - Keypad > - Switching between Firmware and Flash mode > - - Framebuffer for monochrome 256x64 display > - - Backlight control (needs CONFIG_BACKLIGHT_CLASS_DEVICE) > - - Contrast control (needs CONFIG_LCD_CLASS_DEVICE) > - - General purpose outputs (needs CONFIG_LEDS_CLASS) > - EEProm / Flash access (via debugfs) > + Features to selectively enable: > + - Framebuffer for monochrome 256x64 display > + - Backlight control > + - Contrast control > + - General purpose outputs > Features that are not (yet) supported: > - IR > > +config HID_PICOLCD_FB > + bool "Framebuffer support" > + default !EMBEDDED > + depends on HID_PICOLCD > + depends on HID_PICOLCD=FB || FB=y > + select FB_DEFERRED_IO > + select FB_SYS_FILLRECT > + select FB_SYS_COPYAREA > + select FB_SYS_IMAGEBLIT > + select FB_SYS_FOPS Could we perhaps also make the sub-choices for individual features availabel only if !EMBEDDED as well? It's probably too much to ask for a single device during oldconfig run, for example ... Thanks, -- Jiri Kosina SUSE Labs, Novell Inc. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Bruno Prémont on 11 Apr 2010 06:20 HID_PICOLCD should depend on LCD_CLASS_DEVICE, otherwise the build fails when HID_PICOLCD=y and LCD_CLASS_DEVICE=m: hid-picolcd.c:(.text+0x84523f): undefined reference to `lcd_device_unregister' hid-picolcd.c:(.text+0x8478ab): undefined reference to `lcd_device_register' hid-picolcd.c:(.text+0x84c15f): undefined reference to `lcd_device_unregister' Same applies to FB, BACKLIGHT_CLASS_DEVICE and LEDS_CLASS. Add suboptions for those features to handle the deps on kbuild side and just check HID_PICOLCD_* in the code. Reported-by: Randy Dunlap <randy.dunlap(a)oracle.com> Signed-off-by: Bruno Prémont <bonbons(a)linux-vserver.org> --- On Thu, 08 April 2010 Jiri Kosina <jkosina(a)suse.cz> wrote: > > +config HID_PICOLCD_FB > > + bool "Framebuffer support" > > + default !EMBEDDED > > + depends on HID_PICOLCD > > + depends on HID_PICOLCD=FB || FB=y > > + select FB_DEFERRED_IO > > + select FB_SYS_FILLRECT > > + select FB_SYS_COPYAREA > > + select FB_SYS_IMAGEBLIT > > + select FB_SYS_FOPS > > Could we perhaps also make the sub-choices for individual features > availabel only if !EMBEDDED as well? Here is an updated patch to make the sub-choices visible only in EMBEDDED case. Thanks, Bruno drivers/hid/Kconfig | 53 +++++++++++++++++++++++++++++++++++++------- drivers/hid/hid-picolcd.c | 40 +++++++++++++++++----------------- 2 files changed, 64 insertions(+), 29 deletions(-) diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig index a2ecd83..621c52c 100644 --- a/drivers/hid/Kconfig +++ b/drivers/hid/Kconfig @@ -265,11 +265,6 @@ config HID_PETALYNX config HID_PICOLCD tristate "PicoLCD (graphic version)" depends on USB_HID - select FB_DEFERRED_IO if FB - select FB_SYS_FILLRECT if FB - select FB_SYS_COPYAREA if FB - select FB_SYS_IMAGEBLIT if FB - select FB_SYS_FOPS if FB ---help--- This provides support for Minibox PicoLCD devices, currently only the graphical ones are supported. @@ -277,14 +272,54 @@ config HID_PICOLCD This includes support for the following device features: - Keypad - Switching between Firmware and Flash mode - - Framebuffer for monochrome 256x64 display - - Backlight control (needs CONFIG_BACKLIGHT_CLASS_DEVICE) - - Contrast control (needs CONFIG_LCD_CLASS_DEVICE) - - General purpose outputs (needs CONFIG_LEDS_CLASS) - EEProm / Flash access (via debugfs) + Features selectively enabled: + - Framebuffer for monochrome 256x64 display + - Backlight control + - Contrast control + - General purpose outputs Features that are not (yet) supported: - IR +config HID_PICOLCD_FB + bool "Framebuffer support" if EMBEDDED + default !EMBEDDED + depends on HID_PICOLCD + depends on HID_PICOLCD=FB || FB=y + select FB_DEFERRED_IO + select FB_SYS_FILLRECT + select FB_SYS_COPYAREA + select FB_SYS_IMAGEBLIT + select FB_SYS_FOPS + ---help--- + Provide access to PicoLCD's 256x64 monochrome display via a + frambuffer device. + +config HID_PICOLCD_BACKLIGHT + bool "Backlight control" if EMBEDDED + default !EMBEDDED + depends on HID_PICOLCD + depends on HID_PICOLCD=BACKLIGHT_CLASS_DEVICE || BACKLIGHT_CLASS_DEVICE=y + ---help--- + Provide access to PicoLCD's backlight control via backlight + class. + +config HID_PICOLCD_LCD + bool "Contrast control" if EMBEDDED + default !EMBEDDED + depends on HID_PICOLCD + depends on HID_PICOLCD=LCD_CLASS_DEVICE || LCD_CLASS_DEVICE=y + ---help--- + Provide access to PicoLCD's LCD contrast via lcd class. + +config HID_PICOLCD_LEDS + bool "GPO via leds class" if EMBEDDED + default !EMBEDDED + depends on HID_PICOLCD + depends on HID_PICOLCD=LEDS_CLASS || LEDS_CLASS=y + ---help--- + Provide access to PicoLCD's GPO pins via leds class. + config HID_QUANTA tristate "Quanta Optical Touch" depends on USB_HID diff --git a/drivers/hid/hid-picolcd.c b/drivers/hid/hid-picolcd.c index 0eacc6b..0fbc7d3 100644 --- a/drivers/hid/hid-picolcd.c +++ b/drivers/hid/hid-picolcd.c @@ -77,7 +77,7 @@ #define REPORT_HOOK_VERSION 0xf7 /* LCD: IN[2], OUT[1] */ #define REPORT_EXIT_FLASHER 0xff /* Bootloader: OUT[2] */ -#if defined(CONFIG_FB) || defined(CONFIG_FB_MODULE) +#ifdef CONFIG_HID_PICOLCD_FB /* Framebuffer * * The PicoLCD use a Topway LCD module of 256x64 pixel @@ -128,7 +128,7 @@ static const struct fb_var_screeninfo picolcdfb_var = { .bits_per_pixel = 1, .grayscale = 1, }; -#endif /* CONFIG_FB */ +#endif /* CONFIG_HID_PICOLCD_FB */ /* Input device * @@ -183,7 +183,7 @@ struct picolcd_data { struct input_dev *input_cir; unsigned short keycode[PICOLCD_KEYS]; -#if defined(CONFIG_FB) || defined(CONFIG_FB_MODULE) +#ifdef CONFIG_HID_PICOLCD_FB /* Framebuffer stuff */ u8 fb_update_rate; u8 fb_bpp; @@ -191,21 +191,21 @@ struct picolcd_data { u8 *fb_bitmap; /* framebuffer */ struct fb_info *fb_info; struct fb_deferred_io fb_defio; -#endif /* CONFIG_FB */ -#if defined(CONFIG_LCD_CLASS_DEVICE) || defined(CONFIG_LCD_CLASS_DEVICE_MODULE) +#endif /* CONFIG_HID_PICOLCD_FB */ +#ifdef CONFIG_HID_PICOLCD_LCD struct lcd_device *lcd; u8 lcd_contrast; -#endif -#if defined(CONFIG_BACKLIGHT_CLASS_DEVICE) || defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE) +#endif /* CONFIG_HID_PICOLCD_LCD */ +#ifdef CONFIG_HID_PICOLCD_BACKLIGHT struct backlight_device *backlight; u8 lcd_brightness; u8 lcd_power; -#endif /* CONFIG_BACKLIGHT_CLASS_DEVICE */ -#if defined(CONFIG_LEDS_CLASS) || defined(CONFIG_LEDS_CLASS_MODULE) +#endif /* CONFIG_HID_PICOLCD_BACKLIGHT */ +#ifdef CONFIG_HID_PICOLCD_LEDS /* LED stuff */ u8 led_state; struct led_classdev *led[8]; -#endif /* CONFIG_LEDS_CLASS */ +#endif /* CONFIG_HID_PICOLCD_LEDS */ /* Housekeeping stuff */ spinlock_t lock; @@ -287,7 +287,7 @@ static struct picolcd_pending *picolcd_send_and_wait(struct hid_device *hdev, return work; } -#if defined(CONFIG_FB) || defined(CONFIG_FB_MODULE) +#ifdef CONFIG_HID_PICOLCD_FB /* Send a given tile to PicoLCD */ static int picolcd_fb_send_tile(struct hid_device *hdev, int chip, int tile) { @@ -766,9 +766,9 @@ static void picolcd_exit_framebuffer(struct picolcd_data *data) { } #define picolcd_fbinfo(d) NULL -#endif /* CONFIG_FB */ +#endif /* CONFIG_HID_PICOLCD_FB */ -#if defined(CONFIG_BACKLIGHT_CLASS_DEVICE) || defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE) +#ifdef CONFIG_HID_PICOLCD_BACKLIGHT /* * backlight class device */ @@ -864,9 +864,9 @@ static inline int picolcd_resume_backlight(struct picolcd_data *data) { return 0; } -#endif /* CONFIG_BACKLIGHT_CLASS_DEVICE */ +#endif /* CONFIG_HID_PICOLCD_BACKLIGHT */ -#if defined(CONFIG_LCD_CLASS_DEVICE) || defined(CONFIG_LCD_CLASS_DEVICE_MODULE) +#ifdef CONFIG_HID_PICOLCD_LCD /* * lcd class device */ @@ -957,9 +957,9 @@ static inline int picolcd_resume_lcd(struct picolcd_data *data) { return 0; } -#endif /* CONFIG_LCD_CLASS_DEVICE */ +#endif /* CONFIG_HID_PICOLCD_LCD */ -#if defined(CONFIG_LEDS_CLASS) || defined(CONFIG_LEDS_CLASS_MODULE) +#ifdef CONFIG_HID_PICOLCD_LEDS /** * LED class device */ @@ -1104,7 +1104,7 @@ static inline int picolcd_leds_set(struct picolcd_data *data) { return 0; } -#endif /* CONFIG_LEDS_CLASS */ +#endif /* CONFIG_HID_PICOLCD_LEDS */ /* * input class device @@ -1243,10 +1243,10 @@ static int picolcd_reset(struct hid_device *hdev) picolcd_resume_lcd(data); picolcd_resume_backlight(data); -#if defined(CONFIG_FB) || defined(CONFIG_FB_MODULE) +#ifdef CONFIG_HID_PICOLCD_FB if (data->fb_info) schedule_delayed_work(&data->fb_info->deferred_work, 0); -#endif /* CONFIG_FB */ +#endif /* CONFIG_HID_PICOLCD_FB */ picolcd_leds_set(data); return 0; -- 1.6.4.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Jiri Kosina on 11 Apr 2010 14:30
On Sun, 11 Apr 2010, Bruno Prémont wrote: > > > +config HID_PICOLCD_FB > > > + bool "Framebuffer support" > > > + default !EMBEDDED > > > + depends on HID_PICOLCD > > > + depends on HID_PICOLCD=FB || FB=y > > > + select FB_DEFERRED_IO > > > + select FB_SYS_FILLRECT > > > + select FB_SYS_COPYAREA > > > + select FB_SYS_IMAGEBLIT > > > + select FB_SYS_FOPS > > > > Could we perhaps also make the sub-choices for individual features > > availabel only if !EMBEDDED as well? > > Here is an updated patch to make the sub-choices visible only in > EMBEDDED case. Applied, thanks Bruno. -- Jiri Kosina SUSE Labs, Novell Inc. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |