2010-02-20 23:30:27

by Bruno Prémont

[permalink] [raw]
Subject: [Patch 0/3] backlight

This series improves backlight_ops's check_fb function, marks
backlight_ops as const and fixes error checking when registering
backlight device.

Bruno


2010-02-20 23:30:42

by Bruno Prémont

[permalink] [raw]
Subject: [PATCH 2/3] backlight: mark struct backlight_ops const


Signed-off-by: Bruno Prémont <[email protected]>
---
drivers/acpi/video.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_backlight.c | 4 ++--
drivers/macintosh/via-pmu-backlight.c | 4 ++--
drivers/platform/x86/acer-wmi.c | 2 +-
drivers/platform/x86/asus-laptop.c | 2 +-
drivers/platform/x86/asus_acpi.c | 2 +-
drivers/platform/x86/classmate-laptop.c | 2 +-
drivers/platform/x86/compal-laptop.c | 2 +-
drivers/platform/x86/dell-laptop.c | 2 +-
drivers/platform/x86/eeepc-laptop.c | 2 +-
drivers/platform/x86/fujitsu-laptop.c | 2 +-
drivers/platform/x86/msi-laptop.c | 2 +-
drivers/platform/x86/msi-wmi.c | 2 +-
drivers/platform/x86/panasonic-laptop.c | 2 +-
drivers/platform/x86/sony-laptop.c | 2 +-
drivers/platform/x86/thinkpad_acpi.c | 2 +-
drivers/platform/x86/toshiba_acpi.c | 2 +-
drivers/staging/samsung-laptop/samsung-laptop.c | 2 +-
drivers/usb/misc/appledisplay.c | 2 +-
drivers/video/atmel_lcdfb.c | 2 +-
drivers/video/aty/aty128fb.c | 2 +-
drivers/video/aty/atyfb_base.c | 2 +-
drivers/video/aty/radeon_backlight.c | 2 +-
drivers/video/bf54x-lq043fb.c | 2 +-
drivers/video/bfin-t350mcqb-fb.c | 2 +-
drivers/video/nvidia/nv_backlight.c | 2 +-
drivers/video/omap2/displays/panel-taal.c | 2 +-
drivers/video/riva/fbdev.c | 2 +-
28 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index b765790..c86c401 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -366,7 +366,7 @@ static int acpi_video_set_brightness(struct backlight_device *bd)
vd->brightness->levels[request_level]);
}

-static struct backlight_ops acpi_backlight_ops = {
+static const struct backlight_ops acpi_backlight_ops = {
.get_brightness = acpi_video_get_brightness,
.update_status = acpi_video_set_brightness,
};
diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c b/drivers/gpu/drm/nouveau/nouveau_backlight.c
index 20564f8..9a8c990 100644
--- a/drivers/gpu/drm/nouveau/nouveau_backlight.c
+++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c
@@ -58,7 +58,7 @@ static int nv40_set_intensity(struct backlight_device *bd)
return 0;
}

-static struct backlight_ops nv40_bl_ops = {
+static const struct backlight_ops nv40_bl_ops = {
.options = BL_CORE_SUSPENDRESUME,
.get_brightness = nv40_get_intensity,
.update_status = nv40_set_intensity,
@@ -81,7 +81,7 @@ static int nv50_set_intensity(struct backlight_device *bd)
return 0;
}

-static struct backlight_ops nv50_bl_ops = {
+static const struct backlight_ops nv50_bl_ops = {
.options = BL_CORE_SUSPENDRESUME,
.get_brightness = nv50_get_intensity,
.update_status = nv50_set_intensity,
diff --git a/drivers/macintosh/via-pmu-backlight.c b/drivers/macintosh/via-pmu-backlight.c
index a348bb0..ecd9b3f 100644
--- a/drivers/macintosh/via-pmu-backlight.c
+++ b/drivers/macintosh/via-pmu-backlight.c
@@ -15,7 +15,7 @@

#define MAX_PMU_LEVEL 0xFF

-static struct backlight_ops pmu_backlight_data;
+static const struct backlight_ops pmu_backlight_data;
static DEFINE_SPINLOCK(pmu_backlight_lock);
static int sleeping, uses_pmu_bl;
static u8 bl_curve[FB_BACKLIGHT_LEVELS];
@@ -115,7 +115,7 @@ static int pmu_backlight_get_brightness(struct backlight_device *bd)
return bd->props.brightness;
}

-static struct backlight_ops pmu_backlight_data = {
+static const struct backlight_ops pmu_backlight_data = {
.get_brightness = pmu_backlight_get_brightness,
.update_status = pmu_backlight_update_status,

diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
index 07d14df..f70bd41 100644
--- a/drivers/platform/x86/acer-wmi.c
+++ b/drivers/platform/x86/acer-wmi.c
@@ -915,7 +915,7 @@ static int update_bl_status(struct backlight_device *bd)
return 0;
}

-static struct backlight_ops acer_bl_ops = {
+static const struct backlight_ops acer_bl_ops = {
.get_brightness = read_brightness,
.update_status = update_bl_status,
};
diff --git a/drivers/platform/x86/asus-laptop.c b/drivers/platform/x86/asus-laptop.c
index 61a1c75..88e8554 100644
--- a/drivers/platform/x86/asus-laptop.c
+++ b/drivers/platform/x86/asus-laptop.c
@@ -251,7 +251,7 @@ static struct backlight_device *asus_backlight_device;
*/
static int read_brightness(struct backlight_device *bd);
static int update_bl_status(struct backlight_device *bd);
-static struct backlight_ops asusbl_ops = {
+static const struct backlight_ops asusbl_ops = {
.get_brightness = read_brightness,
.update_status = update_bl_status,
};
diff --git a/drivers/platform/x86/asus_acpi.c b/drivers/platform/x86/asus_acpi.c
index c1d2aee..9980d51 100644
--- a/drivers/platform/x86/asus_acpi.c
+++ b/drivers/platform/x86/asus_acpi.c
@@ -1464,7 +1464,7 @@ static int asus_hotk_remove(struct acpi_device *device, int type)
return 0;
}

-static struct backlight_ops asus_backlight_data = {
+static const struct backlight_ops asus_backlight_data = {
.get_brightness = read_brightness,
.update_status = set_brightness_status,
};
diff --git a/drivers/platform/x86/classmate-laptop.c b/drivers/platform/x86/classmate-laptop.c
index ed90082..bfae789 100644
--- a/drivers/platform/x86/classmate-laptop.c
+++ b/drivers/platform/x86/classmate-laptop.c
@@ -452,7 +452,7 @@ static int cmpc_bl_update_status(struct backlight_device *bd)
return -1;
}

-static struct backlight_ops cmpc_bl_ops = {
+static const struct backlight_ops cmpc_bl_ops = {
.get_brightness = cmpc_bl_get_brightness,
.update_status = cmpc_bl_update_status
};
diff --git a/drivers/platform/x86/compal-laptop.c b/drivers/platform/x86/compal-laptop.c
index 1a387e7..2bd6c65 100644
--- a/drivers/platform/x86/compal-laptop.c
+++ b/drivers/platform/x86/compal-laptop.c
@@ -162,7 +162,7 @@ static int bl_update_status(struct backlight_device *b)
return set_lcd_level(b->props.brightness);
}

-static struct backlight_ops compalbl_ops = {
+static const struct backlight_ops compalbl_ops = {
.get_brightness = bl_get_brightness,
.update_status = bl_update_status,
};
diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index 3780994..55640e5 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -333,7 +333,7 @@ static int dell_get_intensity(struct backlight_device *bd)
return buffer.output[1];
}

-static struct backlight_ops dell_ops = {
+static const struct backlight_ops dell_ops = {
.get_brightness = dell_get_intensity,
.update_status = dell_send_intensity,
};
diff --git a/drivers/platform/x86/eeepc-laptop.c b/drivers/platform/x86/eeepc-laptop.c
index e2be6bb..4f77774 100644
--- a/drivers/platform/x86/eeepc-laptop.c
+++ b/drivers/platform/x86/eeepc-laptop.c
@@ -1096,7 +1096,7 @@ static int update_bl_status(struct backlight_device *bd)
return set_brightness(bd, bd->props.brightness);
}

-static struct backlight_ops eeepcbl_ops = {
+static const struct backlight_ops eeepcbl_ops = {
.get_brightness = read_brightness,
.update_status = update_bl_status,
};
diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
index 5f3320d..b6a0941 100644
--- a/drivers/platform/x86/fujitsu-laptop.c
+++ b/drivers/platform/x86/fujitsu-laptop.c
@@ -436,7 +436,7 @@ static int bl_update_status(struct backlight_device *b)
return ret;
}

-static struct backlight_ops fujitsubl_ops = {
+static const struct backlight_ops fujitsubl_ops = {
.get_brightness = bl_get_brightness,
.update_status = bl_update_status,
};
diff --git a/drivers/platform/x86/msi-laptop.c b/drivers/platform/x86/msi-laptop.c
index 759763d..1093ba2 100644
--- a/drivers/platform/x86/msi-laptop.c
+++ b/drivers/platform/x86/msi-laptop.c
@@ -161,7 +161,7 @@ static int bl_update_status(struct backlight_device *b)
return set_lcd_level(b->props.brightness);
}

-static struct backlight_ops msibl_ops = {
+static const struct backlight_ops msibl_ops = {
.get_brightness = bl_get_brightness,
.update_status = bl_update_status,
};
diff --git a/drivers/platform/x86/msi-wmi.c b/drivers/platform/x86/msi-wmi.c
index f5f70d4..5f7cff1 100644
--- a/drivers/platform/x86/msi-wmi.c
+++ b/drivers/platform/x86/msi-wmi.c
@@ -138,7 +138,7 @@ static int bl_set_status(struct backlight_device *bd)
return msi_wmi_set_block(0, backlight_map[bright]);
}

-static struct backlight_ops msi_backlight_ops = {
+static const struct backlight_ops msi_backlight_ops = {
.get_brightness = bl_get,
.update_status = bl_set_status,
};
diff --git a/drivers/platform/x86/panasonic-laptop.c b/drivers/platform/x86/panasonic-laptop.c
index fe7cf01..9012d8d 100644
--- a/drivers/platform/x86/panasonic-laptop.c
+++ b/drivers/platform/x86/panasonic-laptop.c
@@ -352,7 +352,7 @@ static int bl_set_status(struct backlight_device *bd)
return acpi_pcc_write_sset(pcc, SINF_DC_CUR_BRIGHT, bright);
}

-static struct backlight_ops pcc_backlight_ops = {
+static const struct backlight_ops pcc_backlight_ops = {
.get_brightness = bl_get,
.update_status = bl_set_status,
};
diff --git a/drivers/platform/x86/sony-laptop.c b/drivers/platform/x86/sony-laptop.c
index 3f71a60..75481e6 100644
--- a/drivers/platform/x86/sony-laptop.c
+++ b/drivers/platform/x86/sony-laptop.c
@@ -853,7 +853,7 @@ static int sony_backlight_get_brightness(struct backlight_device *bd)
}

static struct backlight_device *sony_backlight_device;
-static struct backlight_ops sony_backlight_ops = {
+static const struct backlight_ops sony_backlight_ops = {
.update_status = sony_backlight_update_status,
.get_brightness = sony_backlight_get_brightness,
};
diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index e67e4fe..950a2f7 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -6112,7 +6112,7 @@ static void tpacpi_brightness_notify_change(void)
BACKLIGHT_UPDATE_HOTKEY);
}

-static struct backlight_ops ibm_backlight_data = {
+static const struct backlight_ops ibm_backlight_data = {
.get_brightness = brightness_get,
.update_status = brightness_update_status,
};
diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
index 77bf5d8..0f0b69e 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -706,7 +706,7 @@ static acpi_status remove_device(void)
return AE_OK;
}

-static struct backlight_ops toshiba_backlight_data = {
+static const struct backlight_ops toshiba_backlight_data = {
.get_brightness = get_lcd,
.update_status = set_lcd_status,
};
diff --git a/drivers/staging/samsung-laptop/samsung-laptop.c b/drivers/staging/samsung-laptop/samsung-laptop.c
index 4877138..0cf2d58 100644
--- a/drivers/staging/samsung-laptop/samsung-laptop.c
+++ b/drivers/staging/samsung-laptop/samsung-laptop.c
@@ -268,7 +268,7 @@ static int update_status(struct backlight_device *bd)
return 0;
}

-static struct backlight_ops backlight_ops = {
+static const struct backlight_ops backlight_ops = {
.get_brightness = get_brightness,
.update_status = update_status,
};
diff --git a/drivers/usb/misc/appledisplay.c b/drivers/usb/misc/appledisplay.c
index 1eb9e41..ef79ca2 100644
--- a/drivers/usb/misc/appledisplay.c
+++ b/drivers/usb/misc/appledisplay.c
@@ -179,7 +179,7 @@ static int appledisplay_bl_get_brightness(struct backlight_device *bd)
return pdata->msgdata[1];
}

-static struct backlight_ops appledisplay_bl_data = {
+static const struct backlight_ops appledisplay_bl_data = {
.get_brightness = appledisplay_bl_get_brightness,
.update_status = appledisplay_bl_update_status,
};
diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
index 3d886c6..22ee7f8 100644
--- a/drivers/video/atmel_lcdfb.c
+++ b/drivers/video/atmel_lcdfb.c
@@ -110,7 +110,7 @@ static int atmel_bl_get_brightness(struct backlight_device *bl)
return lcdc_readl(sinfo, ATMEL_LCDC_CONTRAST_VAL);
}

-static struct backlight_ops atmel_lcdc_bl_ops = {
+static const struct backlight_ops atmel_lcdc_bl_ops = {
.update_status = atmel_bl_update_status,
.get_brightness = atmel_bl_get_brightness,
};
diff --git a/drivers/video/aty/aty128fb.c b/drivers/video/aty/aty128fb.c
index e4e4d43..137cd34 100644
--- a/drivers/video/aty/aty128fb.c
+++ b/drivers/video/aty/aty128fb.c
@@ -1787,7 +1787,7 @@ static int aty128_bl_get_brightness(struct backlight_device *bd)
return bd->props.brightness;
}

-static struct backlight_ops aty128_bl_data = {
+static const struct backlight_ops aty128_bl_data = {
.get_brightness = aty128_bl_get_brightness,
.update_status = aty128_bl_update_status,
};
diff --git a/drivers/video/aty/atyfb_base.c b/drivers/video/aty/atyfb_base.c
index 1ddeb4c..fc4e221 100644
--- a/drivers/video/aty/atyfb_base.c
+++ b/drivers/video/aty/atyfb_base.c
@@ -2225,7 +2225,7 @@ static int aty_bl_get_brightness(struct backlight_device *bd)
return bd->props.brightness;
}

-static struct backlight_ops aty_bl_data = {
+static const struct backlight_ops aty_bl_data = {
.get_brightness = aty_bl_get_brightness,
.update_status = aty_bl_update_status,
};
diff --git a/drivers/video/aty/radeon_backlight.c b/drivers/video/aty/radeon_backlight.c
index 1a056ad..221bd6a 100644
--- a/drivers/video/aty/radeon_backlight.c
+++ b/drivers/video/aty/radeon_backlight.c
@@ -127,7 +127,7 @@ static int radeon_bl_get_brightness(struct backlight_device *bd)
return bd->props.brightness;
}

-static struct backlight_ops radeon_bl_data = {
+static const struct backlight_ops radeon_bl_data = {
.get_brightness = radeon_bl_get_brightness,
.update_status = radeon_bl_update_status,
};
diff --git a/drivers/video/bf54x-lq043fb.c b/drivers/video/bf54x-lq043fb.c
index e49ae5e..db4e6f7 100644
--- a/drivers/video/bf54x-lq043fb.c
+++ b/drivers/video/bf54x-lq043fb.c
@@ -463,7 +463,7 @@ static int bl_get_brightness(struct backlight_device *bd)
return 0;
}

-static struct backlight_ops bfin_lq043fb_bl_ops = {
+static const struct backlight_ops bfin_lq043fb_bl_ops = {
.get_brightness = bl_get_brightness,
};

diff --git a/drivers/video/bfin-t350mcqb-fb.c b/drivers/video/bfin-t350mcqb-fb.c
index 2549c53..bc204c6 100644
--- a/drivers/video/bfin-t350mcqb-fb.c
+++ b/drivers/video/bfin-t350mcqb-fb.c
@@ -381,7 +381,7 @@ static int bl_get_brightness(struct backlight_device *bd)
return 0;
}

-static struct backlight_ops bfin_lq043fb_bl_ops = {
+static const struct backlight_ops bfin_lq043fb_bl_ops = {
.get_brightness = bl_get_brightness,
};

diff --git a/drivers/video/nvidia/nv_backlight.c b/drivers/video/nvidia/nv_backlight.c
index 443e3c8..c443d6a 100644
--- a/drivers/video/nvidia/nv_backlight.c
+++ b/drivers/video/nvidia/nv_backlight.c
@@ -87,7 +87,7 @@ static int nvidia_bl_get_brightness(struct backlight_device *bd)
return bd->props.brightness;
}

-static struct backlight_ops nvidia_bl_ops = {
+static const struct backlight_ops nvidia_bl_ops = {
.get_brightness = nvidia_bl_get_brightness,
.update_status = nvidia_bl_update_status,
};
diff --git a/drivers/video/omap2/displays/panel-taal.c b/drivers/video/omap2/displays/panel-taal.c
index 1f01dfc..a5f7f82 100644
--- a/drivers/video/omap2/displays/panel-taal.c
+++ b/drivers/video/omap2/displays/panel-taal.c
@@ -313,7 +313,7 @@ static int taal_bl_get_intensity(struct backlight_device *dev)
return 0;
}

-static struct backlight_ops taal_bl_ops = {
+static const struct backlight_ops taal_bl_ops = {
.get_brightness = taal_bl_get_intensity,
.update_status = taal_bl_update_status,
};
diff --git a/drivers/video/riva/fbdev.c b/drivers/video/riva/fbdev.c
index d94c57f..912984c 100644
--- a/drivers/video/riva/fbdev.c
+++ b/drivers/video/riva/fbdev.c
@@ -331,7 +331,7 @@ static int riva_bl_get_brightness(struct backlight_device *bd)
return bd->props.brightness;
}

-static struct backlight_ops riva_bl_ops = {
+static const struct backlight_ops riva_bl_ops = {
.get_brightness = riva_bl_get_brightness,
.update_status = riva_bl_update_status,
};
--
1.6.4.4

2010-02-20 23:30:40

by Bruno Prémont

[permalink] [raw]
Subject: [PATCH 1/3] backlight: Add backlight_device parameter to check_fb

check_fb from backlight_ops lacks a reference to the backlight_device
that's being referred to. Add this parameter so a backlight_device
can be mapped to a single framebuffer, especially if the same driver
handles multiple devices on a single system.

Signed-off-by: Bruno Prémont <[email protected]>
---
drivers/video/backlight/adx_bl.c | 2 +-
drivers/video/backlight/backlight.c | 2 +-
include/linux/backlight.h | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/video/backlight/adx_bl.c b/drivers/video/backlight/adx_bl.c
index d769b0b..a683dd1 100644
--- a/drivers/video/backlight/adx_bl.c
+++ b/drivers/video/backlight/adx_bl.c
@@ -56,7 +56,7 @@ static int adx_backlight_get_brightness(struct backlight_device *bldev)
return brightness & 0xff;
}

-static int adx_backlight_check_fb(struct fb_info *fb)
+static int adx_backlight_check_fb(struct backlight_device *bldev, struct fb_info *fb)
{
return 1;
}
diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index 18829cf..b800cd4 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -38,7 +38,7 @@ static int fb_notifier_callback(struct notifier_block *self,
mutex_lock(&bd->ops_lock);
if (bd->ops)
if (!bd->ops->check_fb ||
- bd->ops->check_fb(evdata->info)) {
+ bd->ops->check_fb(bd, evdata->info)) {
bd->props.fb_blank = *(int *)evdata->data;
if (bd->props.fb_blank == FB_BLANK_UNBLANK)
bd->props.state &= ~BL_CORE_FBBLANK;
diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index 8c4f884..15413ce 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -47,7 +47,7 @@ struct backlight_ops {
int (* const get_brightness)(struct backlight_device *);
/* Check if given framebuffer device is the one bound to this backlight;
return 0 if not, !=0 if it is. If NULL, backlight always matches the fb. */
- int (* const check_fb)(struct fb_info *);
+ int (* const check_fb)(struct backlight_device *, struct fb_info *);
};

/* This structure defines all the properties of a backlight */
--
1.6.4.4

2010-02-20 23:30:29

by Bruno Prémont

[permalink] [raw]
Subject: [PATCH 3/3] backlight: fix missing/incomplete registration failure handling

Check newly registered backlight_device for error and properly
return error to parent

Signed-off-by: Bruno Prémont <[email protected]>
---
drivers/platform/x86/classmate-laptop.c | 2 ++
drivers/platform/x86/msi-wmi.c | 4 +++-
drivers/platform/x86/panasonic-laptop.c | 4 +++-
drivers/usb/misc/appledisplay.c | 1 +
drivers/video/bf54x-lq043fb.c | 8 ++++++++
drivers/video/bfin-t350mcqb-fb.c | 8 ++++++++
6 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/classmate-laptop.c b/drivers/platform/x86/classmate-laptop.c
index bfae789..66f6aad 100644
--- a/drivers/platform/x86/classmate-laptop.c
+++ b/drivers/platform/x86/classmate-laptop.c
@@ -463,6 +463,8 @@ static int cmpc_bl_add(struct acpi_device *acpi)

bd = backlight_device_register("cmpc_bl", &acpi->dev,
acpi->handle, &cmpc_bl_ops);
+ if (IS_ERR(bd))
+ return PTR_ERR(bd);
bd->props.max_brightness = 7;
dev_set_drvdata(&acpi->dev, bd);
return 0;
diff --git a/drivers/platform/x86/msi-wmi.c b/drivers/platform/x86/msi-wmi.c
index 5f7cff1..2ffbfcf 100644
--- a/drivers/platform/x86/msi-wmi.c
+++ b/drivers/platform/x86/msi-wmi.c
@@ -251,8 +251,10 @@ static int __init msi_wmi_init(void)
if (!acpi_video_backlight_support()) {
backlight = backlight_device_register(DRV_NAME,
NULL, NULL, &msi_backlight_ops);
- if (IS_ERR(backlight))
+ if (IS_ERR(backlight)) {
+ err = PTR_ERR(backlight);
goto err_free_input;
+ }

backlight->props.max_brightness = ARRAY_SIZE(backlight_map) - 1;
err = bl_get(NULL);
diff --git a/drivers/platform/x86/panasonic-laptop.c b/drivers/platform/x86/panasonic-laptop.c
index 9012d8d..9b17343 100644
--- a/drivers/platform/x86/panasonic-laptop.c
+++ b/drivers/platform/x86/panasonic-laptop.c
@@ -644,8 +644,10 @@ static int acpi_pcc_hotkey_add(struct acpi_device *device)
/* initialize backlight */
pcc->backlight = backlight_device_register("panasonic", NULL, pcc,
&pcc_backlight_ops);
- if (IS_ERR(pcc->backlight))
+ if (IS_ERR(pcc->backlight)) {
+ result = PTR_ERR(pcc->backlight);
goto out_input;
+ }

if (!acpi_pcc_retrieve_biosdata(pcc, pcc->sinf)) {
ACPI_DEBUG_PRINT((ACPI_DB_ERROR,
diff --git a/drivers/usb/misc/appledisplay.c b/drivers/usb/misc/appledisplay.c
index ef79ca2..413f154 100644
--- a/drivers/usb/misc/appledisplay.c
+++ b/drivers/usb/misc/appledisplay.c
@@ -283,6 +283,7 @@ static int appledisplay_probe(struct usb_interface *iface,
&appledisplay_bl_data);
if (IS_ERR(pdata->bd)) {
dev_err(&iface->dev, "Backlight registration failed\n");
+ retval = PTR_ERR(pdata->bd);
goto error;
}

diff --git a/drivers/video/bf54x-lq043fb.c b/drivers/video/bf54x-lq043fb.c
index db4e6f7..a589216 100644
--- a/drivers/video/bf54x-lq043fb.c
+++ b/drivers/video/bf54x-lq043fb.c
@@ -678,6 +678,12 @@ static int __devinit bfin_bf54x_probe(struct platform_device *pdev)
bl_dev =
backlight_device_register("bf54x-bl", NULL, NULL,
&bfin_lq043fb_bl_ops);
+ if (IS_ERR(bl_dev)) {
+ printk(KERN_ERR DRIVER_NAME
+ ": unable to register backlight.\n");
+ ret = -EINVAL;
+ goto out9;
+ }
bl_dev->props.max_brightness = 255;

lcd_dev = lcd_device_register(DRIVER_NAME, &pdev->dev, NULL, &bfin_lcd_ops);
@@ -686,6 +692,8 @@ static int __devinit bfin_bf54x_probe(struct platform_device *pdev)

return 0;

+out9:
+ unregister_framebuffer(fbinfo);
out8:
free_irq(info->irq, info);
out7:
diff --git a/drivers/video/bfin-t350mcqb-fb.c b/drivers/video/bfin-t350mcqb-fb.c
index bc204c6..fe1492b 100644
--- a/drivers/video/bfin-t350mcqb-fb.c
+++ b/drivers/video/bfin-t350mcqb-fb.c
@@ -572,6 +572,12 @@ static int __devinit bfin_t350mcqb_probe(struct platform_device *pdev)
bl_dev =
backlight_device_register("bf52x-bl", NULL, NULL,
&bfin_lq043fb_bl_ops);
+ if (IS_ERR(bl_dev)) {
+ printk(KERN_ERR DRIVER_NAME
+ ": unable to register backlight.\n");
+ ret = -EINVAL;
+ goto out9;
+ }
bl_dev->props.max_brightness = 255;

lcd_dev = lcd_device_register(DRIVER_NAME, NULL, &bfin_lcd_ops);
@@ -580,6 +586,8 @@ static int __devinit bfin_t350mcqb_probe(struct platform_device *pdev)

return 0;

+out9:
+ unregister_framebuffer(fbinfo);
out8:
free_irq(info->irq, info);
out7:
--
1.6.4.4

2010-02-21 09:11:54

by Harald Welte

[permalink] [raw]
Subject: Re: [PATCH 3/3] backlight: fix missing/incomplete registration failure handling

On Sun, Feb 21, 2010 at 12:28:31AM +0100, Bruno Pr?mont wrote:
> Check newly registered backlight_device for error and properly
> return error to parent
>
> Signed-off-by: Bruno Pr?mont <[email protected]>

Acked-by: Harald Welte <[email protected]>

(for panasonic-laptop.c)
--
- Harald Welte <[email protected]> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)

Subject: Re: [PATCH 3/3] backlight: fix missing/incomplete registration failure handling

On Sun, Feb 21, 2010 at 12:28:31AM +0100, Bruno Prémont wrote:
> Check newly registered backlight_device for error and properly
> return error to parent
>
> Signed-off-by: Bruno Prémont <[email protected]>
> ---
> drivers/platform/x86/classmate-laptop.c | 2 ++
> drivers/platform/x86/msi-wmi.c | 4 +++-
> drivers/platform/x86/panasonic-laptop.c | 4 +++-
> drivers/usb/misc/appledisplay.c | 1 +
> drivers/video/bf54x-lq043fb.c | 8 ++++++++
> drivers/video/bfin-t350mcqb-fb.c | 8 ++++++++
> 6 files changed, 25 insertions(+), 2 deletions(-)
>

I think you should split the patch for every driver. Then, every
mantainer may ack only its particular section of the patch.

> diff --git a/drivers/platform/x86/classmate-laptop.c b/drivers/platform/x86/classmate-laptop.c
> index bfae789..66f6aad 100644
> --- a/drivers/platform/x86/classmate-laptop.c
> +++ b/drivers/platform/x86/classmate-laptop.c
> @@ -463,6 +463,8 @@ static int cmpc_bl_add(struct acpi_device *acpi)
>
> bd = backlight_device_register("cmpc_bl", &acpi->dev,
> acpi->handle, &cmpc_bl_ops);
> + if (IS_ERR(bd))
> + return PTR_ERR(bd);
> bd->props.max_brightness = 7;
> dev_set_drvdata(&acpi->dev, bd);
> return 0;
> diff --git a/drivers/platform/x86/msi-wmi.c b/drivers/platform/x86/msi-wmi.c
> index 5f7cff1..2ffbfcf 100644
> --- a/drivers/platform/x86/msi-wmi.c
> +++ b/drivers/platform/x86/msi-wmi.c
> @@ -251,8 +251,10 @@ static int __init msi_wmi_init(void)
> if (!acpi_video_backlight_support()) {
> backlight = backlight_device_register(DRV_NAME,
> NULL, NULL, &msi_backlight_ops);
> - if (IS_ERR(backlight))
> + if (IS_ERR(backlight)) {
> + err = PTR_ERR(backlight);
> goto err_free_input;
> + }
>
> backlight->props.max_brightness = ARRAY_SIZE(backlight_map) - 1;
> err = bl_get(NULL);
> diff --git a/drivers/platform/x86/panasonic-laptop.c b/drivers/platform/x86/panasonic-laptop.c
> index 9012d8d..9b17343 100644
> --- a/drivers/platform/x86/panasonic-laptop.c
> +++ b/drivers/platform/x86/panasonic-laptop.c
> @@ -644,8 +644,10 @@ static int acpi_pcc_hotkey_add(struct acpi_device *device)
> /* initialize backlight */
> pcc->backlight = backlight_device_register("panasonic", NULL, pcc,
> &pcc_backlight_ops);
> - if (IS_ERR(pcc->backlight))
> + if (IS_ERR(pcc->backlight)) {
> + result = PTR_ERR(pcc->backlight);
> goto out_input;
> + }
>
> if (!acpi_pcc_retrieve_biosdata(pcc, pcc->sinf)) {
> ACPI_DEBUG_PRINT((ACPI_DB_ERROR,
> diff --git a/drivers/usb/misc/appledisplay.c b/drivers/usb/misc/appledisplay.c
> index ef79ca2..413f154 100644
> --- a/drivers/usb/misc/appledisplay.c
> +++ b/drivers/usb/misc/appledisplay.c
> @@ -283,6 +283,7 @@ static int appledisplay_probe(struct usb_interface *iface,
> &appledisplay_bl_data);
> if (IS_ERR(pdata->bd)) {
> dev_err(&iface->dev, "Backlight registration failed\n");
> + retval = PTR_ERR(pdata->bd);
> goto error;
> }
>
> diff --git a/drivers/video/bf54x-lq043fb.c b/drivers/video/bf54x-lq043fb.c
> index db4e6f7..a589216 100644
> --- a/drivers/video/bf54x-lq043fb.c
> +++ b/drivers/video/bf54x-lq043fb.c
> @@ -678,6 +678,12 @@ static int __devinit bfin_bf54x_probe(struct platform_device *pdev)
> bl_dev =
> backlight_device_register("bf54x-bl", NULL, NULL,
> &bfin_lq043fb_bl_ops);
> + if (IS_ERR(bl_dev)) {
> + printk(KERN_ERR DRIVER_NAME
> + ": unable to register backlight.\n");
> + ret = -EINVAL;
> + goto out9;
> + }
> bl_dev->props.max_brightness = 255;
>
> lcd_dev = lcd_device_register(DRIVER_NAME, &pdev->dev, NULL, &bfin_lcd_ops);
> @@ -686,6 +692,8 @@ static int __devinit bfin_bf54x_probe(struct platform_device *pdev)
>
> return 0;
>
> +out9:
> + unregister_framebuffer(fbinfo);
> out8:
> free_irq(info->irq, info);
> out7:
> diff --git a/drivers/video/bfin-t350mcqb-fb.c b/drivers/video/bfin-t350mcqb-fb.c
> index bc204c6..fe1492b 100644
> --- a/drivers/video/bfin-t350mcqb-fb.c
> +++ b/drivers/video/bfin-t350mcqb-fb.c
> @@ -572,6 +572,12 @@ static int __devinit bfin_t350mcqb_probe(struct platform_device *pdev)
> bl_dev =
> backlight_device_register("bf52x-bl", NULL, NULL,
> &bfin_lq043fb_bl_ops);
> + if (IS_ERR(bl_dev)) {
> + printk(KERN_ERR DRIVER_NAME
> + ": unable to register backlight.\n");
> + ret = -EINVAL;
> + goto out9;
> + }
> bl_dev->props.max_brightness = 255;
>
> lcd_dev = lcd_device_register(DRIVER_NAME, NULL, &bfin_lcd_ops);
> @@ -580,6 +586,8 @@ static int __devinit bfin_t350mcqb_probe(struct platform_device *pdev)
>
> return 0;
>
> +out9:
> + unregister_framebuffer(fbinfo);
> out8:
> free_irq(info->irq, info);
> out7:
> --
> 1.6.4.4
>


Attachments:
(No filename) (4.65 kB)
signature.asc (198.00 B)
Digital signature
Download all attachments

2010-02-22 19:35:50

by Mike Frysinger

[permalink] [raw]
Subject: Re: [PATCH 2/3] backlight: mark struct backlight_ops const

On Sat, Feb 20, 2010 at 18:18, Bruno Prémont wrote:>  drivers/video/bf54x-lq043fb.c                   |    2 +->  drivers/video/bfin-t350mcqb-fb.c                |    2 +-
Acked-by: Mike Frysinger <[email protected]>-mike????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2010-02-24 15:34:16

by Anisse Astier

[permalink] [raw]
Subject: Re: [PATCH 3/3] backlight: fix missing/incomplete registration failure handling

On Sun, 21 Feb 2010 10:35:29 -0300, Thadeu Lima de Souza Cascardo <[email protected]> wrote :

> On Sun, Feb 21, 2010 at 12:28:31AM +0100, Bruno Prémont wrote:
> > Check newly registered backlight_device for error and properly
> > return error to parent
> >
> > Signed-off-by: Bruno Prémont <[email protected]>
> > ---
> > drivers/platform/x86/classmate-laptop.c | 2 ++
> > drivers/platform/x86/msi-wmi.c | 4 +++-
> > drivers/platform/x86/panasonic-laptop.c | 4 +++-
> > drivers/usb/misc/appledisplay.c | 1 +
> > drivers/video/bf54x-lq043fb.c | 8 ++++++++
> > drivers/video/bfin-t350mcqb-fb.c | 8 ++++++++
> > 6 files changed, 25 insertions(+), 2 deletions(-)
> >
>
> I think you should split the patch for every driver. Then, every
> mantainer may ack only its particular section of the patch.
>

I agree.

> > diff --git a/drivers/platform/x86/msi-wmi.c b/drivers/platform/x86/msi-wmi.c
> > index 5f7cff1..2ffbfcf 100644
> > --- a/drivers/platform/x86/msi-wmi.c
> > +++ b/drivers/platform/x86/msi-wmi.c
> > @@ -251,8 +251,10 @@ static int __init msi_wmi_init(void)
> > if (!acpi_video_backlight_support()) {
> > backlight = backlight_device_register(DRV_NAME,
> > NULL, NULL, &msi_backlight_ops);
> > - if (IS_ERR(backlight))
> > + if (IS_ERR(backlight)) {
> > + err = PTR_ERR(backlight);
> > goto err_free_input;
> > + }
> >
> > backlight->props.max_brightness = ARRAY_SIZE(backlight_map) - 1;
> > err = bl_get(NULL);

Anyway, the msi-wmi part looks good to me.


Anisse

2010-02-24 16:02:28

by Bruno Prémont

[permalink] [raw]
Subject: [PATCH 1/3] picolcd: driver for PicoLCD HID device

Driver for graphic PicoLCD device ( http://www.picolcd.com ) with
framebuffer support (using deferredio), backlight control via backlight
class, contrast control via lcd class and input for keypad.

More features of the device include CIR and GPIO, support for them will
be added at a later time.

Signed-off-by: Bruno Prémont <[email protected]>
Cc: Nicu Pavel <[email protected]>
---
Note, this patch requires
"backlight: Add backlight_device parameter to check_fb"

Some lines are longer than suggested 80 columns, especially those for
the declaration of initial framebuffer content that I did format one
framebuffer line (256 pixel) per source line.

drivers/hid/Kconfig | 18 ++
drivers/hid/Makefile | 1 +
drivers/hid/hid-core.c | 1 +
drivers/hid/hid-ids.h | 1 +
drivers/hid/hid-picolcd.c | 1075 +++++++++++++++++++++++++
5 files changed, 1096 insertions(+), 0 deletions(-)

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 24d90ea..55915c8 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -183,6 +183,24 @@ config LOGIRUMBLEPAD2_FF
Say Y here if you want to enable force feedback support for Logitech
Rumblepad 2 devices.

+config HID_PICOLCD
+ tristate "Minibox PicoLCD (graphic version)"
+ depends on FB
+ select FB_DEFERRED_IO
+ select FB_SYS_FILLRECT
+ select FB_SYS_COPYAREA
+ select FB_SYS_IMAGEBLIT
+ select FB_SYS_FOPS
+ select BACKLIGHT_LCD_SUPPORT
+ select BACKLIGHT_CLASS_DEVICE
+ select LCD_CLASS_DEVICE
+ help
+ This provides support for Minibox PicoLCD devices, currently
+ only the graphical ones are supported. This includes support
+ for the device as a keypad input with mappable keys as well as
+ a framebuffer for the LCD display.
+ Support for the GPIOs and IR is not yet implemented
+
config HID_MICROSOFT
tristate "Microsoft" if EMBEDDED
depends on USB_HID
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 0de2dff..450f09f 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -31,6 +31,7 @@ obj-$(CONFIG_HID_GYRATION) += hid-gyration.o
obj-$(CONFIG_HID_KENSINGTON) += hid-kensington.o
obj-$(CONFIG_HID_KYE) += hid-kye.o
obj-$(CONFIG_HID_LOGITECH) += hid-logitech.o
+obj-$(CONFIG_HID_PICOLCD) += hid-picolcd.o
obj-$(CONFIG_HID_MICROSOFT) += hid-microsoft.o
obj-$(CONFIG_HID_MONTEREY) += hid-monterey.o
obj-$(CONFIG_HID_NTRIG) += hid-ntrig.o
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index eabe5f8..1db0b3a 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1330,6 +1330,7 @@ static const struct hid_device_id hid_blacklist[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_RUMBLEPAD2) },
{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_SPACETRAVELLER) },
{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_SPACENAVIGATOR) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_MICROCHIP, USB_DEVICE_ID_PICOLCD) },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_SIDEWINDER_GV) },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_NE4K) },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_LK6K) },
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 010368e..fc66622 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -330,6 +330,7 @@
#define USB_VENDOR_ID_MICROCHIP 0x04d8
#define USB_DEVICE_ID_PICKIT1 0x0032
#define USB_DEVICE_ID_PICKIT2 0x0033
+#define USB_DEVICE_ID_PICOLCD 0xc002

#define USB_VENDOR_ID_MICROSOFT 0x045e
#define USB_DEVICE_ID_SIDEWINDER_GV 0x003b
diff --git a/drivers/hid/hid-picolcd.c b/drivers/hid/hid-picolcd.c
index e69de29..90f6be5 100644
--- a/drivers/hid/hid-picolcd.c
+++ b/drivers/hid/hid-picolcd.c
@@ -0,0 +1,1075 @@
+/***************************************************************************
+ * Copyright (C) 2010 by Bruno Prémont <[email protected]> *
+ * *
+ * Based on Logitech G13 driver (v0.4) *
+ * Copyright (C) 2009 by Rick L. Vinyard, Jr. <[email protected]> *
+ * *
+ * This program is free software: you can redistribute it and/or modify *
+ * it under the terms of the GNU General Public License as published by *
+ * the Free Software Foundation, either version 2 of the License. *
+ * *
+ * This driver is distributed in the hope that it will be useful, but *
+ * WITHOUT ANY WARRANTY; without even the implied warranty of *
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU *
+ * General Public License for more details. *
+ * *
+ * You should have received a copy of the GNU General Public License *
+ * along with this software. If not see <http://www.gnu.org/licenses/>. *
+ ***************************************************************************/
+
+#include <linux/hid.h>
+#include <linux/input.h>
+#include "hid-ids.h"
+#include "usbhid/usbhid.h"
+#include <linux/usb.h>
+
+#include <linux/fb.h>
+#include <linux/backlight.h>
+#include <linux/lcd.h>
+#include <linux/vmalloc.h>
+
+#include <linux/completion.h>
+
+#define PICOLCD_NAME "PicoLCD (graphic)"
+
+/* Report numbers */
+#define REPORT_BRIGHTNESS 0x91
+#define REPORT_CONTRAST 0x92
+#define REPORT_RESET 0x93
+#define REPORT_LCD_CMD 0x94
+#define REPORT_LCD_DATA 0x95
+#define REPORT_LCD_CMD_DATA 0x96
+#define REPORT_VERSION 0xf1
+#define REPORT_SPLASH_SIZE 0xf6
+#define REPORT_KEYPAD 0x11
+/* Additional features of picoLCD that are not supported yet:
+ * - CIR
+ * - GPIO
+ * - Programming / flashing (full firmware or just splash?)
+ * - interface to ICSP header?
+ */
+
+/* Framebuffer
+ *
+ * The PicoLCD use a Topway LCD module of 256x64 pixel
+ * This display area is tiled over 4 controllers with 8 tiles
+ * each. Each tile has 8x64 pixel, each data byte representing
+ * a 1-bit wide vertical line of the tile.
+ *
+ * The display can be updated at a tile granularity.
+ *
+ * Chip 1 Chip 2 Chip 3 Chip 4
+ * +----------------+----------------+----------------+----------------+
+ * | Tile 1 | Tile 1 | Tile 1 | Tile 1 |
+ * +----------------+----------------+----------------+----------------+
+ * | Tile 2 | Tile 2 | Tile 2 | Tile 2 |
+ * +----------------+----------------+----------------+----------------+
+ * ...
+ * +----------------+----------------+----------------+----------------+
+ * | Tile 8 | Tile 8 | Tile 8 | Tile 8 |
+ * +----------------+----------------+----------------+----------------+
+ */
+#define PICOLCDFB_NAME "picolcdfb"
+#define PICOLCDFB_WIDTH (256)
+#define PICOLCDFB_LINE_LENGTH (256/8)
+#define PICOLCDFB_HEIGHT (64)
+#define PICOLCDFB_SIZE (PICOLCDFB_LINE_LENGTH*PICOLCDFB_HEIGHT)
+
+#define PICOLCDFB_UPDATE_RATE_LIMIT 10
+#define PICOLCDFB_UPDATE_RATE_DEFAULT 2
+
+/* Framebuffer visual structures */
+static const struct fb_fix_screeninfo picolcdfb_fix = {
+ .id = PICOLCDFB_NAME,
+ .type = FB_TYPE_PACKED_PIXELS,
+ .visual = FB_VISUAL_MONO01,
+ .xpanstep = 0,
+ .ypanstep = 0,
+ .ywrapstep = 0,
+ .line_length = PICOLCDFB_LINE_LENGTH,
+ .accel = FB_ACCEL_NONE,
+};
+
+static const struct fb_var_screeninfo picolcdfb_var = {
+ .xres = PICOLCDFB_WIDTH,
+ .yres = PICOLCDFB_HEIGHT,
+ .xres_virtual = PICOLCDFB_WIDTH,
+ .yres_virtual = PICOLCDFB_HEIGHT,
+ .width = 103,
+ .height = 26,
+ .bits_per_pixel = 1,
+};
+
+#ifdef CONFIG_LOGO
+/*
+ * This is a default logo to be loaded upon driver initialization
+ * replacing the default PicoLCD image loaded on device
+ * initialization. This is to provide the user a cue that the
+ * Linux driver is loaded and ready.
+ *
+ * This logo contains the text PicoLCD in the center with one penguin
+ * on each side of the text. The penguins are a 64x64 rendition of
+ * the default framebuffer 80x80 monochrome image scaled down and
+ * cleaned up to retain something that looks a little better than
+ * a simple scaling.
+ *
+ * This logo is a simple xbm image created in GIMP and exported.
+ * To view the image copy the following two #defines plus the
+ * picolcd_bits to an ASCII text file and save with extension
+ * .xbm, then open with GIMP or any other graphical editor
+ * such as eog that recognizes the .xbm format.
+ */
+#define picolcd_width 256
+#define picolcd_height 64
+static const u8 picolcd_bits[PICOLCDFB_SIZE] = {
+ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xf8, 0x1f, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xf8, 0x1f, 0xff, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xe7, 0xe7, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xe7, 0xe7, 0xff, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xdf, 0xf9, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xdf, 0xf9, 0xff, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xbf, 0xfe, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xbf, 0xfe, 0xff, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0x7f, 0xff, 0x7f, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x7f, 0xff, 0x7f, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0x7f, 0xcf, 0xbf, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x7f, 0xcf, 0xbf, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0x7f, 0xdf, 0xbf, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x7f, 0xdf, 0xbf, 0xff, 0xff,
+ 0xff, 0xff, 0xfe, 0xff, 0xff, 0xbf, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xfe, 0xff, 0xff, 0xbf, 0xff, 0xff,
+ 0xff, 0xff, 0xfe, 0xff, 0xff, 0xdf, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xfe, 0xff, 0xff, 0xdf, 0xff, 0xff,
+ 0xff, 0xff, 0xfe, 0xff, 0xff, 0xdf, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xfe, 0xff, 0xff, 0xdf, 0xff, 0xff,
+ 0xff, 0xff, 0xfe, 0xcf, 0x8f, 0xdf, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xfe, 0xcf, 0x8f, 0xdf, 0xff, 0xff,
+ 0xff, 0xff, 0xfe, 0xc7, 0x07, 0xdf, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xfe, 0xc7, 0x07, 0xdf, 0xff, 0xff,
+ 0xff, 0xff, 0xfe, 0xd3, 0x67, 0xdf, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xfe, 0xd3, 0x67, 0xdf, 0xff, 0xff,
+ 0xff, 0xff, 0xfe, 0xfb, 0x73, 0xdf, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xfe, 0xfb, 0x73, 0xdf, 0xff, 0xff,
+ 0xff, 0xff, 0xfe, 0xff, 0xf3, 0xbf, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xfe, 0xff, 0xf3, 0xbf, 0xff, 0xff,
+ 0xff, 0xff, 0xfe, 0xf0, 0x37, 0xbf, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xfe, 0xf0, 0x37, 0xbf, 0xff, 0xff,
+ 0xff, 0xff, 0xfe, 0xc5, 0x0f, 0xdf, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xfe, 0xc5, 0x0f, 0xdf, 0xff, 0xff,
+ 0xff, 0xff, 0xfe, 0xc0, 0x1b, 0xdf, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xfe, 0xc0, 0x1b, 0xdf, 0xff, 0xff,
+ 0xff, 0xff, 0xfe, 0xc0, 0x27, 0xef, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xfe, 0xc0, 0x27, 0xef, 0xff, 0xff,
+ 0xff, 0xff, 0xfe, 0xe0, 0xc7, 0xef, 0xff, 0xff, 0xff, 0xff, 0xff, 0xe3, 0xff, 0xff, 0xff, 0xfe, 0x3f, 0xfe, 0x00, 0x78, 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xfe, 0xe0, 0xc7, 0xef, 0xff, 0xff,
+ 0xff, 0xff, 0xfe, 0xff, 0x1b, 0x67, 0xff, 0xff, 0xff, 0xff, 0xff, 0xe3, 0xff, 0xff, 0xff, 0xfe, 0x3f, 0xfc, 0x00, 0x38, 0x00, 0x7f, 0xff, 0xff, 0xff, 0xff, 0xfe, 0xff, 0x1b, 0x67, 0xff, 0xff,
+ 0xff, 0xff, 0xfe, 0xd0, 0xc3, 0xb7, 0xff, 0xff, 0xff, 0xff, 0xff, 0xe3, 0xff, 0xff, 0xff, 0xfe, 0x3f, 0xfc, 0x00, 0x38, 0x00, 0x7f, 0xff, 0xff, 0xff, 0xff, 0xfe, 0xd0, 0xc3, 0xb7, 0xff, 0xff,
+ 0xff, 0xff, 0xfd, 0xcf, 0x03, 0xfb, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xfe, 0x3f, 0xfc, 0x7e, 0x38, 0xfc, 0x7f, 0xff, 0xff, 0xff, 0xff, 0xfd, 0xcf, 0x03, 0xfb, 0xff, 0xff,
+ 0xff, 0xff, 0xfd, 0xc0, 0x01, 0xfd, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xfe, 0x3f, 0xfc, 0x7f, 0xf8, 0xfc, 0x7f, 0xff, 0xff, 0xff, 0xff, 0xfd, 0xc0, 0x01, 0xfd, 0xff, 0xff,
+ 0xff, 0xff, 0xfb, 0x80, 0x01, 0xfd, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xfe, 0x3f, 0xfc, 0x7f, 0xf8, 0xfc, 0x7f, 0xff, 0xff, 0xff, 0xff, 0xfb, 0x80, 0x01, 0xfd, 0xff, 0xff,
+ 0xff, 0xff, 0xf7, 0x00, 0x00, 0xfe, 0xff, 0xff, 0xff, 0xf1, 0x01, 0xe3, 0xc0, 0x0f, 0x80, 0x1e, 0x3f, 0xfc, 0x7f, 0xf8, 0xfc, 0x7f, 0xff, 0xff, 0xff, 0xff, 0xf7, 0x00, 0x00, 0xfe, 0xff, 0xff,
+ 0xff, 0xff, 0xef, 0x00, 0x00, 0xff, 0x7f, 0xff, 0xff, 0xf0, 0x00, 0xe3, 0x80, 0x07, 0x00, 0x0e, 0x3f, 0xfc, 0x7f, 0xf8, 0xfc, 0x7f, 0xff, 0xff, 0xff, 0xff, 0xef, 0x00, 0x00, 0xff, 0x7f, 0xff,
+ 0xff, 0xff, 0xef, 0x00, 0x00, 0x7f, 0x7f, 0xff, 0xff, 0xf0, 0x00, 0xe3, 0x80, 0x07, 0x00, 0x0e, 0x3f, 0xfc, 0x7f, 0xf8, 0xfc, 0x7f, 0xff, 0xff, 0xff, 0xff, 0xef, 0x00, 0x00, 0x7f, 0x7f, 0xff,
+ 0xff, 0xff, 0xdf, 0x8c, 0x0f, 0x7f, 0xbf, 0xff, 0xff, 0xf1, 0xf8, 0xe3, 0x8f, 0xc7, 0x1f, 0x8e, 0x3f, 0xfc, 0x7f, 0xf8, 0xfc, 0x7f, 0xff, 0xff, 0xff, 0xff, 0xdf, 0x8c, 0x0f, 0x7f, 0xbf, 0xff,
+ 0xff, 0xff, 0xde, 0x08, 0x07, 0xbf, 0xdf, 0xff, 0xff, 0xf1, 0xf8, 0xe3, 0x8f, 0xff, 0x1f, 0x8e, 0x3f, 0xfc, 0x7f, 0xf8, 0xfc, 0x7f, 0xff, 0xff, 0xff, 0xff, 0xde, 0x08, 0x07, 0xbf, 0xdf, 0xff,
+ 0xff, 0xff, 0xbc, 0x00, 0x01, 0xff, 0xdf, 0xff, 0xff, 0xf1, 0xf8, 0xe3, 0x8f, 0xff, 0x1f, 0x8e, 0x3f, 0xfc, 0x7f, 0xf8, 0xfc, 0x7f, 0xff, 0xff, 0xff, 0xff, 0xbc, 0x00, 0x01, 0xff, 0xdf, 0xff,
+ 0xff, 0xff, 0xbc, 0x00, 0x00, 0x7f, 0xdf, 0xff, 0xff, 0xf1, 0xf8, 0xe3, 0x8f, 0xff, 0x1f, 0x8e, 0x3f, 0xfc, 0x7f, 0xf8, 0xfc, 0x7f, 0xff, 0xff, 0xff, 0xff, 0xbc, 0x00, 0x00, 0x7f, 0xdf, 0xff,
+ 0xff, 0xff, 0x78, 0x00, 0x00, 0x1f, 0xef, 0xff, 0xff, 0xf1, 0xf8, 0xe3, 0x8f, 0xff, 0x1f, 0x8e, 0x3f, 0xfc, 0x7f, 0xf8, 0xfc, 0x7f, 0xff, 0xff, 0xff, 0xff, 0x78, 0x00, 0x00, 0x1f, 0xef, 0xff,
+ 0xff, 0xfe, 0xf8, 0x00, 0x00, 0x1d, 0xe7, 0xff, 0xff, 0xf1, 0xf8, 0xe3, 0x8f, 0xc7, 0x1f, 0x8e, 0x3f, 0x1c, 0x7e, 0x38, 0xfc, 0x7f, 0xff, 0xff, 0xff, 0xfe, 0xf8, 0x00, 0x00, 0x1d, 0xe7, 0xff,
+ 0xff, 0xfe, 0xf0, 0x00, 0x00, 0x1d, 0xf7, 0xff, 0xff, 0xf0, 0x00, 0xe3, 0x80, 0x07, 0x00, 0x0e, 0x00, 0x1c, 0x00, 0x38, 0x00, 0x7f, 0xff, 0xff, 0xff, 0xfe, 0xf0, 0x00, 0x00, 0x1d, 0xf7, 0xff,
+ 0xff, 0xfd, 0xf0, 0x00, 0x00, 0x1d, 0xf7, 0xff, 0xff, 0xf0, 0x00, 0xe3, 0x80, 0x07, 0x00, 0x0e, 0x00, 0x1c, 0x00, 0x38, 0x00, 0x7f, 0xff, 0xff, 0xff, 0xfd, 0xf0, 0x00, 0x00, 0x1d, 0xf7, 0xff,
+ 0xff, 0xfd, 0xf0, 0x00, 0x00, 0x0e, 0xfb, 0xff, 0xff, 0xf1, 0x01, 0xe3, 0xc0, 0x0f, 0x80, 0x1e, 0x00, 0x1e, 0x00, 0x78, 0x00, 0xff, 0xff, 0xff, 0xff, 0xfd, 0xf0, 0x00, 0x00, 0x0e, 0xfb, 0xff,
+ 0xff, 0xfd, 0xe0, 0x10, 0x00, 0x0e, 0xfb, 0xff, 0xff, 0xf1, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xfd, 0xe0, 0x10, 0x00, 0x0e, 0xfb, 0xff,
+ 0xff, 0xfb, 0xe0, 0x10, 0x00, 0x0e, 0xfb, 0xff, 0xff, 0xf1, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xf9, 0x86, 0x7f, 0x3b, 0xff, 0xff, 0xff, 0xff, 0xfb, 0xe0, 0x10, 0x00, 0x0e, 0xfb, 0xff,
+ 0xff, 0xfb, 0xe0, 0x10, 0x00, 0x0e, 0xfb, 0xff, 0xff, 0xf1, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xf6, 0xbd, 0xfe, 0xfb, 0xff, 0xff, 0xff, 0xff, 0xfb, 0xe0, 0x10, 0x00, 0x0e, 0xfb, 0xff,
+ 0xff, 0xfb, 0xe0, 0x10, 0x00, 0x0e, 0xfb, 0xff, 0xff, 0xf1, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xfe, 0x8c, 0x6a, 0x36, 0xff, 0xff, 0xff, 0xff, 0xfb, 0xe0, 0x10, 0x00, 0x0e, 0xfb, 0xff,
+ 0xff, 0xfb, 0xe0, 0x10, 0x00, 0x10, 0x3b, 0xff, 0xff, 0xf1, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xfd, 0xf5, 0xb6, 0xd0, 0xff, 0xff, 0xff, 0xff, 0xfb, 0xe0, 0x10, 0x00, 0x10, 0x3b, 0xff,
+ 0xff, 0xfa, 0x30, 0x00, 0x00, 0x7f, 0xdb, 0xff, 0xff, 0xf1, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xfb, 0xf5, 0xaa, 0xde, 0xff, 0xff, 0xff, 0xff, 0xfa, 0x30, 0x00, 0x00, 0x7f, 0xdb, 0xff,
+ 0xff, 0xfc, 0x38, 0x00, 0x00, 0x4f, 0xe7, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xf0, 0x8e, 0x7f, 0x3e, 0xff, 0xff, 0xff, 0xff, 0xfc, 0x38, 0x00, 0x00, 0x4f, 0xe7, 0xff,
+ 0xff, 0xf8, 0x1c, 0x00, 0x00, 0x8f, 0xc7, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xf8, 0x1c, 0x00, 0x00, 0x8f, 0xc7, 0xff,
+ 0xff, 0xf0, 0x07, 0x00, 0x00, 0x87, 0x87, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xf0, 0x07, 0x00, 0x00, 0x87, 0x87, 0xff,
+ 0xff, 0xc0, 0x07, 0x80, 0x00, 0x80, 0x07, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xc0, 0x07, 0x80, 0x00, 0x80, 0x07, 0xff,
+ 0xff, 0x00, 0x03, 0xc0, 0x00, 0x80, 0x07, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00, 0x03, 0xc0, 0x00, 0x80, 0x07, 0xff,
+ 0xff, 0x00, 0x03, 0xc0, 0x00, 0x80, 0x03, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00, 0x03, 0xc0, 0x00, 0x80, 0x03, 0xff,
+ 0xff, 0x00, 0x01, 0xc0, 0x00, 0x80, 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00, 0x01, 0xc0, 0x00, 0x80, 0x00, 0xff,
+ 0xff, 0x00, 0x01, 0xc0, 0x01, 0x80, 0x00, 0x7f, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00, 0x01, 0xc0, 0x01, 0x80, 0x00, 0x7f,
+ 0xff, 0x00, 0x00, 0x80, 0x03, 0x80, 0x00, 0x7f, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00, 0x00, 0x80, 0x03, 0x80, 0x00, 0x7f,
+ 0xff, 0x00, 0x00, 0x40, 0x07, 0x80, 0x00, 0x7f, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00, 0x00, 0x40, 0x07, 0x80, 0x00, 0x7f,
+ 0xff, 0x00, 0x00, 0x00, 0x3f, 0x80, 0x01, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00, 0x00, 0x00, 0x3f, 0x80, 0x01, 0xff,
+ 0xff, 0x00, 0x00, 0x3f, 0xff, 0x00, 0x03, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00, 0x00, 0x3f, 0xff, 0x00, 0x03, 0xff,
+ 0xff, 0x00, 0x00, 0x3f, 0xff, 0x00, 0x0f, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00, 0x00, 0x3f, 0xff, 0x00, 0x0f, 0xff,
+ 0xff, 0x80, 0x00, 0x3f, 0xff, 0x00, 0x1f, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x80, 0x00, 0x3f, 0xff, 0x00, 0x1f, 0xff,
+ 0xff, 0xfc, 0x00, 0x38, 0x03, 0x00, 0x7f, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xfc, 0x00, 0x38, 0x03, 0x00, 0x7f, 0xff,
+ 0xff, 0xff, 0xc0, 0x07, 0xfc, 0x80, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xc0, 0x07, 0xfc, 0x80, 0xff, 0xff,
+ 0xff, 0xff, 0xe0, 0x7f, 0xff, 0x81, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xe0, 0x7f, 0xff, 0x81, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff
+};
+#endif
+
+
+/* Input device
+ *
+ * The PicoLCD has an IR receiver header, a built-in keypad with 5 keys
+ * and header for 4x4 key matrix. The built-in keys are part of the matrix.
+ */
+#define PICOLCD_KEYS 33
+
+static const int def_keymap[PICOLCD_KEYS] = {
+ KEY_RESERVED, KEY_BACK, KEY_HOMEPAGE, KEY_RESERVED, KEY_RESERVED, KEY_SCROLLUP, KEY_OK, KEY_SCROLLDOWN,
+ KEY_RESERVED, KEY_RESERVED, KEY_RESERVED, KEY_RESERVED, KEY_RESERVED, KEY_RESERVED, KEY_RESERVED, KEY_RESERVED,
+ KEY_RESERVED, KEY_RESERVED, KEY_RESERVED, KEY_RESERVED, KEY_RESERVED, KEY_RESERVED, KEY_RESERVED, KEY_RESERVED,
+ KEY_RESERVED, KEY_RESERVED, KEY_RESERVED, KEY_RESERVED, KEY_RESERVED, KEY_RESERVED, KEY_RESERVED, KEY_RESERVED,
+ KEY_RESERVED
+};
+
+
+
+/* Per device data structure */
+struct picolcd_data {
+ struct hid_device *hdev;
+
+ /* input stuff */
+ struct input_dev *input_keys;
+ struct input_dev *input_cir;
+ int keycode[PICOLCD_KEYS];
+ u8 pressed_keys[2];
+
+ /* Framebuffer stuff */
+ u8 fb_update_rate;
+ u8 *fb_vbitmap; /* local copy of what was sent to PicoLCD */
+ u8 *fb_bitmap; /* framebuffer */
+ struct fb_info *fb_info;
+ struct fb_deferred_io fb_defio;
+ struct lcd_device *lcd;
+ struct backlight_device *backlight;
+ int lcd_contrast;
+ int lcd_power;
+ int lcd_brightness;
+
+ /* GPIO stuff */
+
+ /* Housekeeping stuff */
+ spinlock_t lock;
+ struct completion ready;
+ int status;
+#define PICOLCD_READY_ALIVE 1
+#define PICOLCD_READY_FB 2
+#define PICOLCD_READY_ALL (PICOLCD_READY_ALIVE | PICOLCD_READY_FB)
+};
+
+
+
+/* Find a given output report */
+static struct hid_report *picolcd_out_report(int id, struct hid_device *hdev)
+{
+ struct list_head *feature_report_list = &hdev->report_enum[HID_OUTPUT_REPORT].report_list;
+ struct hid_report *report = NULL;
+
+ list_for_each_entry(report, feature_report_list, list) {
+ if (report->id == id)
+ return report;
+ }
+ dev_err(&hdev->dev, "No report with id 0x%x found\n", id);
+ return NULL;
+}
+
+
+
+/* Send a given tile to PicoLCD */
+static inline int picolcd_fb_send_tile(struct hid_device *hdev, int chip,
+ int tile)
+{
+ struct picolcd_data *data = hid_get_drvdata(hdev);
+ struct hid_report *report1 = picolcd_out_report(REPORT_LCD_CMD_DATA, hdev);
+ struct hid_report *report2 = picolcd_out_report(REPORT_LCD_DATA, hdev);
+ u8 *tdata;
+ int i;
+
+ if (!report1 || report1->maxfield != 1 || !report2 || report2->maxfield != 1)
+ return -ENODEV;
+
+ hid_set_field(report1->field[0], 0, chip << 2);
+ hid_set_field(report1->field[0], 1, 0x02);
+ hid_set_field(report1->field[0], 2, 0x00);
+ hid_set_field(report1->field[0], 3, 0x00);
+ hid_set_field(report1->field[0], 4, 0xb8 | tile);
+ hid_set_field(report1->field[0], 5, 0x00);
+ hid_set_field(report1->field[0], 6, 0x00);
+ hid_set_field(report1->field[0], 7, 0x40);
+ hid_set_field(report1->field[0], 8, 0x00);
+ hid_set_field(report1->field[0], 9, 0x00);
+ hid_set_field(report1->field[0], 10, 32);
+
+ hid_set_field(report2->field[0], 0, (chip << 2) | 0x01);
+ hid_set_field(report2->field[0], 1, 0x00);
+ hid_set_field(report2->field[0], 2, 0x00);
+ hid_set_field(report2->field[0], 3, 32);
+
+ tdata = data->fb_vbitmap + (tile * 4 + chip) * 64;
+ for (i = 0; i < 64; i++)
+ if (i < 32)
+ hid_set_field(report1->field[0], 11 + i, tdata[i]);
+ else
+ hid_set_field(report2->field[0], 4 + i - 32, tdata[i]);
+
+ usbhid_submit_report(data->hdev, report1, USB_DIR_OUT);
+ usbhid_submit_report(data->hdev, report2, USB_DIR_OUT);
+ return 0;
+}
+
+/* Translate a single tile*/
+static inline int picolcd_fb_update_tile(u8 *vbitmap, const u8 *bitmap,
+ int chip, int tile)
+{
+ int i, b, changed = 0;
+ u8 tdata[64];
+ u8 *vdata = vbitmap + (tile * 4 + chip) * 64;
+
+ for (b = 7; b >= 0; b--) {
+ const u8 *bdata = bitmap + tile * 256 + chip * 8 + b * 32;
+ for (i = 0; i < 64; i++) {
+ tdata[i] <<= 1;
+ tdata[i] |= (bdata[i/8] >> (7 - i % 8)) & 0x01;
+ }
+ }
+ for (i = 0; i < 64; i++)
+ if (tdata[i] != vdata[i]) {
+ changed = 1;
+ vdata[i] = tdata[i];
+ }
+ return changed;
+}
+
+/* Reconfigure LCD display
+ * TODO: prevent concurrent runs */
+static int picolcd_fb_reset(struct hid_device *hdev, int clear)
+{
+ struct picolcd_data *data = hid_get_drvdata(hdev);
+ struct hid_report *report = picolcd_out_report(REPORT_LCD_CMD, hdev);
+ int chip, tile, i, j;
+ static const u8 mapcmd[8] = { 0x00, 0x02, 0x00, 0x64, 0x3f, 0x00, 0x64, 0xc0 };
+
+ if (!report || report->maxfield != 1)
+ return -ENODEV;
+
+ for (i = 0; i < 4; i++) {
+ for (j = 0; j < report->field[0]->maxusage; j++)
+ if (j == 0)
+ hid_set_field(report->field[0], j, i << 2);
+ else if (j < sizeof(mapcmd))
+ hid_set_field(report->field[0], j, mapcmd[j]);
+ else
+ hid_set_field(report->field[0], j, 0);
+ usbhid_submit_report(data->hdev, report, USB_DIR_OUT);
+ }
+
+ if (!clear) {
+ spin_lock(&data->lock);
+ data->status |= PICOLCD_READY_FB;
+ spin_unlock(&data->lock);
+ for (chip = 0; chip < 4; chip++)
+ for (tile = 0; tile < 8; tile++) {
+ i = picolcd_fb_send_tile(data->hdev, chip, tile);
+ if (i != 0)
+ return i;
+ }
+ return 0;
+ }
+
+#ifdef CONFIG_LOGO
+ memcpy(data->fb_bitmap, picolcd_bits, PICOLCDFB_SIZE);
+#else
+ memset(data->fb_bitmap, 0, PICOLCDFB_SIZE);
+#endif
+ for (chip = 0; chip < 4; chip++)
+ for (tile = 0; tile < 8; tile++) {
+ picolcd_fb_update_tile(data->fb_vbitmap,
+ data->fb_bitmap, chip, tile);
+ i = picolcd_fb_send_tile(data->hdev, chip, tile);
+ if (i != 0)
+ return i;
+ }
+
+ spin_lock(&data->lock);
+ data->status |= PICOLCD_READY_FB;
+ spin_unlock(&data->lock);
+ return 0;
+}
+
+/* Update fb_vbitmap from the screen_base and send changed tiles to device */
+static void picolcd_fb_update(struct picolcd_data *data)
+{
+ int chip, tile;
+
+ spin_lock(&data->lock);
+ if (!(data->status & PICOLCD_READY_FB)) {
+ spin_unlock(&data->lock);
+ picolcd_fb_reset(data->hdev, 0);
+ } else
+ spin_unlock(&data->lock);
+
+ /*
+ * Translate the XBM format screen_base into the format needed by the
+ * PicoLCD. See display layout above.
+ * Do this one tile after the other and push those tiles that changed.
+ */
+ for (chip = 0; chip < 4; chip++)
+ for (tile = 0; tile < 8; tile++)
+ if (picolcd_fb_update_tile(data->fb_vbitmap,
+ data->fb_bitmap, chip, tile))
+ picolcd_fb_send_tile(data->hdev, chip, tile);
+}
+
+/* Stub to call the system default and update the image on the picoLCD */
+static void picolcd_fb_fillrect(struct fb_info *info,
+ const struct fb_fillrect *rect)
+{
+ if (!info->par)
+ return;
+ sys_fillrect(info, rect);
+
+ picolcd_fb_update(info->par);
+}
+
+/* Stub to call the system default and update the image on the picoLCD */
+static void picolcd_fb_copyarea(struct fb_info *info,
+ const struct fb_copyarea *area)
+{
+ if (!info->par)
+ return;
+ sys_copyarea(info, area);
+
+ picolcd_fb_update(info->par);
+}
+
+/* Stub to call the system default and update the image on the picoLCD */
+static void picolcd_fb_imageblit(struct fb_info *info, const struct fb_image *image)
+{
+ if (!info->par)
+ return;
+ sys_imageblit(info, image);
+
+ picolcd_fb_update(info->par);
+}
+
+/*
+ * this is the slow path from userspace. they can seek and write to
+ * the fb. it's inefficient to do anything less than a full screen draw
+ */
+static ssize_t picolcd_fb_write(struct fb_info *info, const char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ ssize_t ret;
+ if (!info->par)
+ return -ENODEV;
+ ret = fb_sys_write(info, buf, count, ppos);
+ if (ret >= 0)
+ picolcd_fb_update(info->par);
+ return ret;
+}
+
+static int picolcd_fb_blank(int blank, struct fb_info *info)
+{
+ if (!info->par)
+ return -ENODEV;
+ /* We let fb notification do this for us via lcd/backlight device */
+ return 0;
+}
+
+static void picolcd_fb_destroy(struct fb_info *info)
+{
+ fb_deferred_io_cleanup(info);
+ if (info->par)
+ ((struct picolcd_data *)info->par)->fb_info = NULL;
+ framebuffer_release(info);
+}
+
+/* Note this can't be const because of struct fb_info definition */
+static struct fb_ops picolcdfb_ops = {
+ .owner = THIS_MODULE,
+ .fb_destroy = picolcd_fb_destroy,
+ .fb_read = fb_sys_read,
+ .fb_write = picolcd_fb_write,
+ .fb_blank = picolcd_fb_blank,
+ .fb_fillrect = picolcd_fb_fillrect,
+ .fb_copyarea = picolcd_fb_copyarea,
+ .fb_imageblit = picolcd_fb_imageblit,
+};
+
+
+/* Callback from deferred IO workqueue */
+static void picolcd_fb_deferred_io(struct fb_info *info, struct list_head *pagelist)
+{
+ picolcd_fb_update(info->par);
+}
+
+static const struct fb_deferred_io picolcd_fb_defio = {
+ .delay = HZ / PICOLCDFB_UPDATE_RATE_DEFAULT,
+ .deferred_io = picolcd_fb_deferred_io,
+};
+
+
+/*
+ * The "fb_update_rate" attribute
+ */
+static ssize_t picolcd_fb_update_rate_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct picolcd_data *data = dev_get_drvdata(dev);
+ unsigned fb_update_rate = data->fb_update_rate;
+
+ return snprintf(buf, PAGE_SIZE, "%u (1..%u)\n", fb_update_rate,
+ PICOLCDFB_UPDATE_RATE_LIMIT);
+}
+
+static ssize_t picolcd_fb_update_rate_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct picolcd_data *data = dev_get_drvdata(dev);
+ int i;
+ unsigned u;
+
+ if (count < 1 || count > 10)
+ return -EINVAL;
+
+ i = sscanf(buf, "%u", &u);
+ if (i != 1)
+ return -EINVAL;
+
+ if (u > PICOLCDFB_UPDATE_RATE_LIMIT)
+ return -ERANGE;
+ else if (u == 0)
+ u = PICOLCDFB_UPDATE_RATE_DEFAULT;
+
+ data->fb_update_rate = u;
+ data->fb_defio.delay = HZ / data->fb_update_rate;
+ return count;
+}
+
+static DEVICE_ATTR(fb_update_rate, 0666, picolcd_fb_update_rate_show,
+ picolcd_fb_update_rate_store);
+
+
+/*
+ * backlight class device
+ */
+static int picolcd_get_brightness(struct backlight_device *bdev)
+{
+ struct picolcd_data *data = bl_get_data(bdev);
+ return data->lcd_brightness;
+}
+
+static int picolcd_set_brightness(struct backlight_device *bdev)
+{
+ struct picolcd_data *data = bl_get_data(bdev);
+ struct hid_report *report = picolcd_out_report(REPORT_BRIGHTNESS, data->hdev);
+
+ if (!report || report->maxfield != 1 || report->field[0]->maxusage != 1)
+ return -ENODEV;
+
+ data->lcd_brightness = bdev->props.brightness & 0x0ff;
+ data->lcd_power = bdev->props.power;
+ hid_set_field(report->field[0], 0, data->lcd_power == FB_BLANK_UNBLANK ? data->lcd_brightness : 0);
+ usbhid_submit_report(data->hdev, report, USB_DIR_OUT);
+ return 0;
+}
+
+static int picolcd_check_bl_fb(struct backlight_device *bdev, struct fb_info *fb)
+{
+ struct picolcd_data *data = bl_get_data(bdev);
+ return fb == data->fb_info;
+}
+
+static const struct backlight_ops picolcd_blops = {
+ .update_status = picolcd_set_brightness,
+ .get_brightness = picolcd_get_brightness,
+ .check_fb = picolcd_check_bl_fb,
+};
+
+/*
+ * lcd class device
+ */
+static int picolcd_get_power(struct lcd_device *ldev)
+{
+ struct picolcd_data *data = lcd_get_data(ldev);
+ return data->lcd_power;
+}
+
+static int picolcd_set_power(struct lcd_device *ldev, int power)
+{
+ struct picolcd_data *data = lcd_get_data(ldev);
+ data->backlight->props.power = power;
+ return picolcd_set_brightness(data->backlight);
+}
+
+static int picolcd_get_contrast(struct lcd_device *ldev)
+{
+ struct picolcd_data *data = lcd_get_data(ldev);
+ return data->lcd_contrast;
+}
+
+static int picolcd_set_contrast(struct lcd_device *ldev, int contrast)
+{
+ struct picolcd_data *data = lcd_get_data(ldev);
+ struct hid_report *report = picolcd_out_report(REPORT_CONTRAST, data->hdev);
+
+ if (!report || report->maxfield != 1 || report->field[0]->maxusage != 1)
+ return -ENODEV;
+
+ data->lcd_contrast = contrast & 0x0ff;
+ hid_set_field(report->field[0], 0, data->lcd_contrast);
+ usbhid_submit_report(data->hdev, report, USB_DIR_OUT);
+ return 0;
+}
+
+static int picolcd_check_lcd_fb(struct lcd_device *ldev, struct fb_info *fb)
+{
+ struct picolcd_data *data = lcd_get_data(ldev);
+ return fb == data->fb_info;
+}
+
+static struct lcd_ops picolcd_lcdops = {
+ .get_power = picolcd_get_power,
+ .set_power = picolcd_set_power,
+ .get_contrast = picolcd_get_contrast,
+ .set_contrast = picolcd_set_contrast,
+ .check_fb = picolcd_check_lcd_fb,
+};
+
+
+/*
+ * input class device
+ */
+static int picolcd_raw_keypad(struct hid_device *hdev,
+ struct hid_report *report, u8 *raw_data, int size)
+{
+ /*
+ * Keypad event
+ * First and second data bytes list currently pressed keys,
+ * 0x00 means no key and at most 2 keys may be pressed at same time
+ */
+ struct picolcd_data *data = hid_get_drvdata(hdev);
+ int i, j;
+
+ /* determine newly pressed keys */
+ for (i = 0; i < size; i++) {
+ int key_code;
+ if (raw_data[i] == 0)
+ continue;
+ for (j = 0; j < sizeof(data->pressed_keys); j++)
+ if (data->pressed_keys[j] == raw_data[i])
+ goto key_already_down;
+ for (j = 0; j < sizeof(data->pressed_keys); j++)
+ if (data->pressed_keys[j] == 0) {
+ data->pressed_keys[j] = raw_data[i];
+ break;
+ }
+ input_event(data->input_keys, EV_MSC, MSC_SCAN, raw_data[i]);
+ if (input_get_keycode(data->input_keys, raw_data[i], &key_code))
+ key_code = KEY_UNKNOWN;
+ if (key_code != KEY_UNKNOWN) {
+ dbg_hid(PICOLCD_NAME " got key press for %u:%d", raw_data[i], key_code);
+ input_report_key(data->input_keys, key_code, 1);
+ }
+ input_sync(data->input_keys);
+key_already_down:
+ continue;
+ }
+
+ /* determine newly released keys */
+ for (j = 0; j < sizeof(data->pressed_keys); j++) {
+ int key_code;
+ if (data->pressed_keys[j] == 0)
+ continue;
+ for (i = 0; i < size; i++)
+ if (data->pressed_keys[j] == raw_data[i])
+ goto key_still_down;
+ input_event(data->input_keys, EV_MSC, MSC_SCAN, data->pressed_keys[j]);
+ if (input_get_keycode(data->input_keys, data->pressed_keys[j], &key_code))
+ key_code = KEY_UNKNOWN;
+ if (key_code != KEY_UNKNOWN) {
+ dbg_hid(PICOLCD_NAME " got key release for %u:%d", data->pressed_keys[j], key_code);
+ input_report_key(data->input_keys, key_code, 0);
+ }
+ input_sync(data->input_keys);
+ data->pressed_keys[j] = 0;
+key_still_down:
+ continue;
+ }
+ return 1;
+}
+
+
+
+
+
+/*
+ * Reset our device and wait for answer to VERSION request
+ */
+static int picolcd_reset(struct hid_device *hdev)
+{
+ struct picolcd_data *data = hid_get_drvdata(hdev);
+ struct hid_report *report1 = picolcd_out_report(REPORT_RESET, hdev);
+ struct hid_report *report2 = picolcd_out_report(REPORT_VERSION, hdev);
+ int ret;
+
+ if (!report1 || report1->maxfield != 1 || !report2)
+ return -ENODEV;
+
+ spin_lock(&data->lock);
+ data->status = 0;
+ spin_unlock(&data->lock);
+ INIT_COMPLETION(data->ready);
+
+ hid_set_field(report1->field[0], 0, 1);
+ usbhid_submit_report(hdev, report1, USB_DIR_OUT);
+
+ usbhid_submit_report(hdev, report2, USB_DIR_OUT);
+ wait_for_completion_interruptible_timeout(&data->ready, HZ*2);
+
+ spin_lock(&data->lock);
+ if (data->status == 0) {
+ spin_unlock(&data->lock);
+ return -EBUSY;
+ } else
+ spin_unlock(&data->lock);
+
+ ret = picolcd_set_contrast(data->lcd, data->lcd_contrast);
+ if (ret)
+ return ret;
+ ret = picolcd_set_brightness(data->backlight);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+/*
+ * The "reset" attribute
+ */
+static ssize_t picolcd_reset_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return snprintf(buf, PAGE_SIZE, "all fb");
+}
+
+static ssize_t picolcd_reset_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct picolcd_data *data = dev_get_drvdata(dev);
+ size_t cnt = count;
+ while (cnt > 0 && (buf[cnt-1] == ' ' || buf[cnt-1] == '\n'))
+ cnt--;
+ if (strncmp(buf, "all", cnt) == 0) {
+ picolcd_reset(data->hdev);
+ picolcd_fb_reset(data->hdev, 1);
+ } else if (strncmp(buf, "fb", cnt) == 0) {
+ picolcd_fb_reset(data->hdev, 1);
+ } else
+ return -EINVAL;
+ return count;
+}
+
+static DEVICE_ATTR(reset, 0600, picolcd_reset_show,
+ picolcd_reset_store);
+
+
+
+/*
+ * Handle raw report as sent by device
+ */
+static int picolcd_raw_event(struct hid_device *hdev,
+ struct hid_report *report, u8 *raw_data, int size)
+{
+ struct picolcd_data *data = hid_get_drvdata(hdev);
+ char hexdata[25];
+ int i;
+
+ if (data == NULL)
+ return 1;
+
+ for (i = 0; i < sizeof(hexdata) / 2; i++)
+ sprintf(hexdata+2*i, "%02hhx", raw_data[i]);
+ if (size >= sizeof(hexdata)/2) {
+ hexdata[sizeof(hexdata)-4] = '.';
+ hexdata[sizeof(hexdata)-3] = '.';
+ hexdata[sizeof(hexdata)-2] = '.';
+ hexdata[sizeof(hexdata)-1] = '\0';
+ } else
+ hexdata[size*2] = '\0';
+
+ switch (report->id) {
+ case REPORT_KEYPAD:
+ if (size == 3 && raw_data[0] == 0x11 && data->input_keys) {
+ return picolcd_raw_keypad(hdev, report, raw_data+1, size-1);
+ } else {
+ dbg_hid(PICOLCD_NAME " unsupported key event (%d bytes): 0x%s\n", size, hexdata);
+ break;
+ }
+ break;
+ case REPORT_VERSION:
+ if (size == 3)
+ dev_info(&hdev->dev, "Firmware version is %hd.%hd\n", raw_data[1], raw_data[2]);
+
+ spin_lock(&data->lock);
+ if (!(data->status & PICOLCD_READY_ALIVE)) {
+ data->status |= PICOLCD_READY_ALIVE;
+ spin_unlock(&data->lock);
+ complete_all(&data->ready);
+ } else
+ spin_unlock(&data->lock);
+ break;
+
+ default:
+ dbg_hid(PICOLCD_NAME " got raw event %d with length %d: %s\n", report->id, size, hexdata);
+ return 0;
+ }
+
+ return 1;
+}
+
+
+/*
+ * Create a group of attributes so that we can create and destroy them all
+ * at once.
+ */
+static struct attribute *picolcd_attrs[] = {
+ &dev_attr_fb_update_rate.attr,
+ &dev_attr_reset.attr,
+ NULL, /* need to NULL terminate the list of attributes */
+};
+
+/*
+ * An unnamed attribute group will put all of the attributes directly in
+ * the kobject directory. If we specify a name, a subdirectory will be
+ * created for the attributes with the directory being the name of the
+ * attribute group.
+ */
+static const struct attribute_group picolcd_attr_group = {
+ .attrs = picolcd_attrs,
+};
+
+
+static int picolcd_probe(struct hid_device *hdev,
+ const struct hid_device_id *id)
+{
+ struct picolcd_data *data;
+ struct input_dev *idev;
+ int i, error = -ENOMEM;
+ char name[12];
+
+ dbg_hid(PICOLCD_NAME " hardware probe...\n");
+
+ /*
+ * Let's allocate the picolcd data structure, set some reasonable
+ * defaults, and associate it with the device
+ */
+ data = kzalloc(sizeof(struct picolcd_data), GFP_KERNEL);
+ if (data == NULL) {
+ dev_err(&hdev->dev, "can't allocate space for Minibox PicoLCD device data\n");
+ error = -ENOMEM;
+ goto err_no_cleanup;
+ }
+
+ spin_lock_init(&data->lock);
+ init_completion(&data->ready);
+ data->hdev = hdev;
+ hid_set_drvdata(hdev, data);
+
+ data->fb_bitmap = vmalloc(PICOLCDFB_SIZE);
+ if (data->fb_bitmap == NULL) {
+ dev_err(&hdev->dev, "can't get a free page for framebuffer\n");
+ error = -ENOMEM;
+ goto err_cleanup_data;
+ }
+
+ data->fb_vbitmap = kmalloc(PICOLCDFB_SIZE, GFP_KERNEL);
+ if (data->fb_vbitmap == NULL) {
+ dev_err(&hdev->dev, "can't alloc vbitmap image buffer\n");
+ error = -ENOMEM;
+ goto err_cleanup_fb_bitmap;
+ }
+
+ /* Parse the device reports and start it up */
+ error = hid_parse(hdev);
+ if (error) {
+ dev_err(&hdev->dev, "device report parse failed\n");
+ goto err_cleanup_fb_vbitmap;
+ }
+
+ error = hid_hw_start(hdev, HID_CONNECT_HIDINPUT);
+ if (error) {
+ dev_err(&hdev->dev, "hardware start failed\n");
+ goto err_cleanup_fb_vbitmap;
+ }
+ dbg_hid(PICOLCD_NAME " claimed: %d\n", hdev->claimed);
+
+ error = hdev->ll_driver->open(hdev);
+ if (error) {
+ dev_err(&hdev->dev, "failed to open input interrupt pipe for key and IR events\n");
+ goto err_cleanup_hid_hw;
+ }
+
+ /* Setup input device */
+ idev = input_allocate_device();
+ if (idev == NULL) {
+ dev_err(&hdev->dev, "failed to allocate input device");
+ error = -ENOMEM;
+ goto err_cleanup_hid_ll;
+ }
+ input_set_drvdata(idev, hdev);
+ memcpy(data->keycode, def_keymap, sizeof(def_keymap));
+ idev->name = hdev->name;
+ idev->phys = hdev->phys;
+ idev->uniq = hdev->uniq;
+ idev->id.bustype = hdev->bus;
+ idev->id.vendor = hdev->vendor;
+ idev->id.product = hdev->product;
+ idev->id.version = hdev->version;
+ idev->dev.parent = hdev->dev.parent;
+ idev->keycode = &data->keycode;
+ idev->keycodemax = PICOLCD_KEYS;
+ idev->keycodesize = sizeof(int);
+ input_set_capability(idev, EV_MSC, MSC_SCAN);
+ set_bit(EV_REP, idev->evbit);
+ for (i = 0; i < PICOLCD_KEYS; i++) {
+ int key = ((int *)idev->keycode)[i];
+ if (key < KEY_MAX && key >= 0)
+ input_set_capability(idev, EV_KEY, key);
+ }
+ error = input_register_device(idev);
+ if (error) {
+ dev_err(&hdev->dev, "error registering the input device");
+ input_free_device(idev);
+ goto err_cleanup_hid_ll;
+ }
+ data->input_keys = idev;
+
+ /* Set up the framebuffer device */
+ data->fb_update_rate = PICOLCDFB_UPDATE_RATE_DEFAULT;
+ data->fb_defio = picolcd_fb_defio;
+ data->fb_info = framebuffer_alloc(0, &hdev->dev);
+ if (data->fb_info == NULL) {
+ dev_err(&hdev->dev, "failed to allocate a framebuffer\n");
+ error = -ENOMEM;
+ goto err_cleanup_input_keys;
+ }
+
+ data->fb_info->fbdefio = &data->fb_defio;
+ data->fb_info->screen_base = (char __force __iomem *) data->fb_bitmap;
+ data->fb_info->fbops = &picolcdfb_ops;
+ data->fb_info->var = picolcdfb_var;
+ data->fb_info->fix = picolcdfb_fix;
+ data->fb_info->fix.smem_len = PICOLCDFB_SIZE;
+ data->fb_info->par = data;
+ data->fb_info->flags = FBINFO_FLAG_DEFAULT;
+ fb_deferred_io_init(data->fb_info);
+
+ error = register_framebuffer(data->fb_info);
+ if (error < 0) {
+ dev_err(&hdev->dev, "failed to register framebuffer\n");
+ fb_deferred_io_cleanup(data->fb_info);
+ framebuffer_release(data->fb_info);
+ goto err_cleanup_input_keys;
+ } else
+ error = 0;
+
+ snprintf(name, sizeof(name), "fb%d", data->fb_info->node);
+ data->lcd = lcd_device_register(name, &hdev->dev, data, &picolcd_lcdops);
+ if (IS_ERR(data->lcd)) {
+ dev_err(&hdev->dev, "failed to register LCD\n");
+ error = PTR_ERR(data->lcd);
+ goto err_cleanup_fb;
+ }
+ data->lcd->props.max_contrast = 0x0ff;
+ data->lcd_contrast = 0xe5;
+
+ data->backlight = backlight_device_register(name, &hdev->dev, data, &picolcd_blops);
+ if (IS_ERR(data->backlight)) {
+ dev_err(&hdev->dev, "failed to register backlight\n");
+ error = PTR_ERR(data->backlight);
+ goto err_cleanup_lcd;
+ }
+ data->backlight->props.max_brightness = 0x0ff;
+ data->backlight->props.brightness = 0xff;
+ data->lcd_brightness = 0xff;
+
+ dbg_hid(PICOLCD_NAME " waiting for device to activate\n");
+ if (!picolcd_reset(hdev)) {
+ dbg_hid(PICOLCD_NAME " initialized\n");
+ } else
+ dev_warn(&hdev->dev, "hasn't responded yet, forging ahead with initialization\n");
+
+ picolcd_fb_reset(hdev, 1);
+
+ /* Add the sysfs attributes */
+ error = sysfs_create_group(&(hdev->dev.kobj), &picolcd_attr_group);
+ if (error) {
+ dev_err(&hdev->dev, "failed to create sysfs group attributes\n");
+ goto err_cleanup_bl;
+ }
+
+ dbg_hid(PICOLCD_NAME " activated and initialized\n");
+ return 0;
+
+
+err_cleanup_bl:
+ backlight_device_unregister(data->backlight);
+err_cleanup_lcd:
+ lcd_device_unregister(data->lcd);
+err_cleanup_fb:
+ unregister_framebuffer(data->fb_info);
+err_cleanup_input_keys:
+ input_unregister_device(data->input_keys);
+err_cleanup_hid_ll:
+ hdev->ll_driver->close(hdev);
+err_cleanup_hid_hw:
+ hid_hw_stop(hdev);
+err_cleanup_fb_vbitmap:
+ kfree(data->fb_vbitmap);
+err_cleanup_fb_bitmap:
+ vfree(data->fb_bitmap);
+err_cleanup_data:
+ kfree(data);
+err_no_cleanup:
+ hid_set_drvdata(hdev, NULL);
+
+ return error;
+}
+
+static void picolcd_remove(struct hid_device *hdev)
+{
+ struct picolcd_data *data = hid_get_drvdata(hdev);
+
+ dbg_hid(PICOLCD_NAME " hardware remove...\n");
+ sysfs_remove_group(&(hdev->dev.kobj), &picolcd_attr_group);
+
+ hdev->ll_driver->close(hdev);
+ hid_hw_stop(hdev);
+
+ /* Clean up the framebuffer */
+ backlight_device_unregister(data->backlight);
+ lcd_device_unregister(data->lcd);
+ unregister_framebuffer(data->fb_info);
+
+ /* Get the internal picolcd data buffer */
+ input_unregister_device(data->input_keys);
+
+ vfree(data->fb_bitmap);
+ kfree(data->fb_vbitmap);
+
+ /* Finally, clean up the picolcd data itself */
+ kfree(data);
+}
+
+static const struct hid_device_id picolcd_devices[] = {
+ { HID_USB_DEVICE(USB_VENDOR_ID_MICROCHIP, USB_DEVICE_ID_PICOLCD) },
+ { }
+};
+MODULE_DEVICE_TABLE(hid, picolcd_devices);
+
+static struct hid_driver picolcd_driver = {
+ .name = "hid-picolcd",
+ .id_table = picolcd_devices,
+ .probe = picolcd_probe,
+ .remove = picolcd_remove,
+ .raw_event = picolcd_raw_event,
+};
+
+static int __init picolcd_init(void)
+{
+ return hid_register_driver(&picolcd_driver);
+}
+
+static void __exit picolcd_exit(void)
+{
+ hid_unregister_driver(&picolcd_driver);
+}
+
+module_init(picolcd_init);
+module_exit(picolcd_exit);
+MODULE_DESCRIPTION("Minibox graphics PicoLCD Driver");
+MODULE_LICENSE("GPL");

2010-02-24 18:28:17

by Oliver Neukum

[permalink] [raw]
Subject: Re: [PATCH 1/3] picolcd: driver for PicoLCD HID device

Am Mittwoch, 24. Februar 2010 17:00:49 schrieb Bruno Prémont:
> +static int picolcd_raw_event(struct hid_device *hdev,
> + struct hid_report *report, u8 *raw_data, int size)
> +{
> + struct picolcd_data *data = hid_get_drvdata(hdev);
> + char hexdata[25];
> + int i;
> +
> + if (data == NULL)
> + return 1;
> +
> + for (i = 0; i < sizeof(hexdata) / 2; i++)
> + sprintf(hexdata+2*i, "%02hhx", raw_data[i]);
> + if (size >= sizeof(hexdata)/2) {
> + hexdata[sizeof(hexdata)-4] = '.';
> + hexdata[sizeof(hexdata)-3] = '.';
> + hexdata[sizeof(hexdata)-2] = '.';
> + hexdata[sizeof(hexdata)-1] = '\0';
> + } else
> + hexdata[size*2] = '\0';
> +
> + switch (report->id) {
> + case REPORT_KEYPAD:
> + if (size == 3 && raw_data[0] == 0x11 && data->input_keys) {
> + return picolcd_raw_keypad(hdev, report, raw_data+1, size-1);
> + } else {
> + dbg_hid(PICOLCD_NAME " unsupported key event (%d bytes): 0x%s\n", size, hexdata);
> + break;
> + }
> + break;
> + case REPORT_VERSION:
> + if (size == 3)
> + dev_info(&hdev->dev, "Firmware version is %hd.%hd\n", raw_data[1], raw_data[2]);
> +
> + spin_lock(&data->lock);

If I recall correctly raw_event is called in interrupt. As you take a spinlock here,
the lock in code not called in interrupt must disable interrupts, or you may
deadlock.

Regards
Oliver

2010-02-24 21:45:12

by Rick L. Vinyard, Jr.

[permalink] [raw]
Subject: Re: [PATCH 1/3] picolcd: driver for PicoLCD HID device


Oliver Neukum wrote:
> Am Mittwoch, 24. Februar 2010 17:00:49 schrieb Bruno Prémont:
>> +static int picolcd_raw_event(struct hid_device *hdev,
>> + struct hid_report *report, u8 *raw_data, int size)
>> +{
>> + struct picolcd_data *data = hid_get_drvdata(hdev);
>> + char hexdata[25];
>> + int i;
>> +
>> + if (data == NULL)
>> + return 1;
>> +
>> + for (i = 0; i < sizeof(hexdata) / 2; i++)
>> + sprintf(hexdata+2*i, "%02hhx", raw_data[i]);
>> + if (size >= sizeof(hexdata)/2) {
>> + hexdata[sizeof(hexdata)-4] = '.';
>> + hexdata[sizeof(hexdata)-3] = '.';
>> + hexdata[sizeof(hexdata)-2] = '.';
>> + hexdata[sizeof(hexdata)-1] = '\0';
>> + } else
>> + hexdata[size*2] = '\0';
>> +
>> + switch (report->id) {
>> + case REPORT_KEYPAD:
>> + if (size == 3 && raw_data[0] == 0x11 &&
>> data->input_keys) {
>> + return picolcd_raw_keypad(hdev, report,
>> raw_data+1, size-1);
>> + } else {
>> + dbg_hid(PICOLCD_NAME " unsupported key event (%d
>> bytes): 0x%s\n", size, hexdata);
>> + break;
>> + }
>> + break;
>> + case REPORT_VERSION:
>> + if (size == 3)
>> + dev_info(&hdev->dev, "Firmware version is
>> %hd.%hd\n", raw_data[1], raw_data[2]);
>> +
>> + spin_lock(&data->lock);
>
> If I recall correctly raw_event is called in interrupt. As you take a
> spinlock here,
> the lock in code not called in interrupt must disable interrupts, or you
> may
> deadlock.
>

The issue, as I understand it is that non-interrupt code may obtain the
lock and then the interrupt code is executed... hence the deadlock and the
need to use spin_lock_irqsave() and spin_unlock_irqrestore().

If that is correct, is there any problem with the following approach?

The key difference is the replacement of spin_lock() with spin_trylock()
such that if the non-interrupt code has already obtained the lock, the
interrupt will not deadlock but instead take the else path and schedule a
framebuffer update at the next interval.

static void g13_fb_urb_completion(struct urb *urb)
{
struct g13_data *data = urb->context;
spin_unlock(&data->fb_urb_lock);
}

static int g13_fb_send(struct hid_device *hdev)
{
struct usb_interface *intf;
struct usb_device *usb_dev;
struct g13_data *data = hid_get_g13data(hdev);

struct usb_host_endpoint *ep;
unsigned int pipe;
int retval = 0;

/*
* Try and lock the framebuffer urb to prevent access if we have
* submitted it. If we can't lock it we'll have to delay this update
* until the next framebuffer interval.
*
* Fortunately, we already have the infrastructure in place with the
* framebuffer deferred I/O driver to schedule the delayed update.
*/
if (likely(spin_trylock(&data->fb_urb_lock))) {
/* Get the usb device to send the image on */
intf = to_usb_interface(hdev->dev.parent);
usb_dev = interface_to_usbdev(intf);

pipe = usb_sndintpipe(usb_dev, 0x02);

ep = usb_dev->ep_out[usb_pipeendpoint(pipe)];

if (unlikely(!ep)) {
spin_unlock(&data->fb_urb_lock);
return -EINVAL;
}

pipe = (pipe & ~(3 << 30)) | (PIPE_INTERRUPT << 30);
usb_fill_int_urb(data->fb_urb, usb_dev, pipe, data->fb_vbitmap,
G13_VBITMAP_SIZE, g13_fb_urb_completion, NULL,
ep->desc.bInterval);
data->fb_urb->context = data;
data->fb_urb->actual_length = 0;

retval = usb_submit_urb(data->fb_urb, GFP_NOIO);
if (unlikely(retval < 0)) {
/*
* We need to unlock the framebuffer urb lock since
* the urb submission failed and therefore
* g13_fb_urb_completion() won't be called.
*/
spin_unlock(&data->fb_urb_lock);
return retval;
}
} else {
schedule_delayed_work(&data->fb_info->deferred_work,
data->fb_defio.delay);
}

return retval;
}

Thanks,

--

Rick

2010-02-25 04:12:19

by Oliver Neukum

[permalink] [raw]
Subject: Re: [PATCH 1/3] picolcd: driver for PicoLCD HID device

Am Mittwoch, 24. Februar 2010 22:44:53 schrieb Rick L. Vinyard, Jr.:
> The issue, as I understand it is that non-interrupt code may obtain the
> lock and then the interrupt code is executed... hence the deadlock and the
> need to use spin_lock_irqsave() and spin_unlock_irqrestore().

Yes.

> If that is correct, is there any problem with the following approach?

Why not always a workqueue or alternatively spin_lock_irq() when
you are not in interrupt? This approach seems needlessly complicated.

Secondly, when you hold a spinlock, you must use GFP_ATOMIC.
GFP_NOIO is insufficient.

Regards
Oliver

2010-02-25 11:00:53

by Jiri Kosina

[permalink] [raw]
Subject: Re: [PATCH 1/3] picolcd: driver for PicoLCD HID device

On Wed, 24 Feb 2010, Rick L. Vinyard, Jr. wrote:

> >> +static int picolcd_raw_event(struct hid_device *hdev,
> >> + struct hid_report *report, u8 *raw_data, int size)
> >> +{
> >> + struct picolcd_data *data = hid_get_drvdata(hdev);
> >> + char hexdata[25];
> >> + int i;
> >> +
> >> + if (data == NULL)
> >> + return 1;
> >> +
> >> + for (i = 0; i < sizeof(hexdata) / 2; i++)
> >> + sprintf(hexdata+2*i, "%02hhx", raw_data[i]);
> >> + if (size >= sizeof(hexdata)/2) {
> >> + hexdata[sizeof(hexdata)-4] = '.';
> >> + hexdata[sizeof(hexdata)-3] = '.';
> >> + hexdata[sizeof(hexdata)-2] = '.';
> >> + hexdata[sizeof(hexdata)-1] = '\0';
> >> + } else
> >> + hexdata[size*2] = '\0';
> >> +
> >> + switch (report->id) {
> >> + case REPORT_KEYPAD:
> >> + if (size == 3 && raw_data[0] == 0x11 &&
> >> data->input_keys) {
> >> + return picolcd_raw_keypad(hdev, report,
> >> raw_data+1, size-1);
> >> + } else {
> >> + dbg_hid(PICOLCD_NAME " unsupported key event (%d
> >> bytes): 0x%s\n", size, hexdata);
> >> + break;
> >> + }
> >> + break;
> >> + case REPORT_VERSION:
> >> + if (size == 3)
> >> + dev_info(&hdev->dev, "Firmware version is
> >> %hd.%hd\n", raw_data[1], raw_data[2]);
> >> +
> >> + spin_lock(&data->lock);
> >
> > If I recall correctly raw_event is called in interrupt.

Yes, that is correct.

> The issue, as I understand it is that non-interrupt code may obtain the
> lock and then the interrupt code is executed... hence the deadlock and
> the need to use spin_lock_irqsave() and spin_unlock_irqrestore().

Exactly. All the spinlocks that are aquired in interrupt code-paths must
be acquired with _irqsave()/_irqrestore() from the non-interrupt code to
prevent exactly this kind of deadlock.
>
> The key difference is the replacement of spin_lock() with spin_trylock()
> such that if the non-interrupt code has already obtained the lock, the
> interrupt will not deadlock but instead take the else path and schedule a
> framebuffer update at the next interval.

Why is _irqsave() and/or deferred work not enough? The aproach with
_trylock() seems to be overly complicated for no good reason (I personally
become very suspicious every time I see code that is using _trylock()).

[ by the way, Rick, are you planning to resubmit the G13 driver with
incorporated feedback from the last review round? ]

Thanks,

--
Jiri Kosina
SUSE Labs, Novell Inc.

2010-02-25 11:07:42

by Jiri Kosina

[permalink] [raw]
Subject: Re: [PATCH 1/3] picolcd: driver for PicoLCD HID device

On Wed, 24 Feb 2010, Bruno Prémont wrote:

> +config HID_PICOLCD
> + tristate "Minibox PicoLCD (graphic version)"
> + depends on FB
> + select FB_DEFERRED_IO
> + select FB_SYS_FILLRECT
> + select FB_SYS_COPYAREA
> + select FB_SYS_IMAGEBLIT
> + select FB_SYS_FOPS
> + select BACKLIGHT_LCD_SUPPORT
> + select BACKLIGHT_CLASS_DEVICE
> + select LCD_CLASS_DEVICE

I guess you need dependency on USB_HID as well, right?

> --- a/drivers/hid/hid-picolcd.c
> +++ b/drivers/hid/hid-picolcd.c
> @@ -0,0 +1,1075 @@
> +/***************************************************************************
> + * Copyright (C) 2010 by Bruno Prémont <[email protected]> *
> + * *
> + * Based on Logitech G13 driver (v0.4) *
> + * Copyright (C) 2009 by Rick L. Vinyard, Jr. <[email protected]> *
> + * *
> + * This program is free software: you can redistribute it and/or modify *
> + * it under the terms of the GNU General Public License as published by *
> + * the Free Software Foundation, either version 2 of the License. *

I support removing the 'or any later' clause, but I think your version has
the grammar wrong :)


> +/* Update fb_vbitmap from the screen_base and send changed tiles to device */
> +static void picolcd_fb_update(struct picolcd_data *data)
> +{
> + int chip, tile;
> +
> + spin_lock(&data->lock);
> + if (!(data->status & PICOLCD_READY_FB)) {
> + spin_unlock(&data->lock);
> + picolcd_fb_reset(data->hdev, 0);
> + } else
> + spin_unlock(&data->lock);

Please put the brackets to the else branch as well.

Also, it'd be great if the framebuffer part would get Ack from someone
more familiar with framebuffer code than me.

Thanks,

--
Jiri Kosina
SUSE Labs, Novell Inc.

2010-02-25 11:32:52

by Bruno Prémont

[permalink] [raw]
Subject: Re: [PATCH 1/3] picolcd: driver for PicoLCD HID device

On Thu, 25 February 2010 Jiri Kosina <[email protected]> wrote:
> On Wed, 24 Feb 2010, Bruno Prémont wrote:
>
> > +config HID_PICOLCD
> > + tristate "Minibox PicoLCD (graphic version)"
> > + depends on FB
> > + select FB_DEFERRED_IO
> > + select FB_SYS_FILLRECT
> > + select FB_SYS_COPYAREA
> > + select FB_SYS_IMAGEBLIT
> > + select FB_SYS_FOPS
> > + select BACKLIGHT_LCD_SUPPORT
> > + select BACKLIGHT_CLASS_DEVICE
> > + select LCD_CLASS_DEVICE
>
> I guess you need dependency on USB_HID as well, right?

Yep, will add

> > --- a/drivers/hid/hid-picolcd.c
> > +++ b/drivers/hid/hid-picolcd.c
> > @@ -0,0 +1,1075 @@
> > +/***************************************************************************
> > + * Copyright (C) 2010 by Bruno Prémont <[email protected]> *
> > + * *
> > + * Based on Logitech G13 driver (v0.4) *
> > + * Copyright (C) 2009 by Rick L. Vinyard, Jr. <[email protected]> *
> > + * *
> > + * This program is free software: you can redistribute it and/or modify *
> > + * it under the terms of the GNU General Public License as published by *
> > + * the Free Software Foundation, either version 2 of the License. *
>
> I support removing the 'or any later' clause, but I think your
> version has the grammar wrong :)

Oops, will fix

> > +/* Update fb_vbitmap from the screen_base and send changed tiles to device */
> > +static void picolcd_fb_update(struct picolcd_data *data)
> > +{
> > + int chip, tile;
> > +
> > + spin_lock(&data->lock);
> > + if (!(data->status & PICOLCD_READY_FB)) {
> > + spin_unlock(&data->lock);
> > + picolcd_fb_reset(data->hdev, 0);
> > + } else
> > + spin_unlock(&data->lock);
>
> Please put the brackets to the else branch as well.

Ok, will add the brackets while switching from spin_(un)lock to
spin_(un)lock_irq{save|restore}.

> Also, it'd be great if the framebuffer part would get Ack from
> someone more familiar with framebuffer code than me.

I would appreciate such one as well, especially regarding the
deferredio part and the more advanced fb use. For now I only tested
read/write from /dev/fbx.


For the two sysfs attributes I currently use, the 'reset' one shall
probably be moved to debugfs (I would like to place it under
/sys/kernel/debug/hid/$device/ next to rdesc and event).

By the way, I'm wondering why event does not list any of the reports
coming from my device though as I understand the code it should be
doing that before my raw_event function gets called...

The one for deferredio refresh rate should ideally go below fb device
(and I guess that might also be of use for other users of deferredio)

Thanks for the review,
Bruno

2010-02-25 15:18:13

by Jiri Kosina

[permalink] [raw]
Subject: Re: [PATCH 1/3] picolcd: driver for PicoLCD HID device

On Thu, 25 Feb 2010, Bruno Pr?mont wrote:

> For the two sysfs attributes I currently use, the 'reset' one shall
> probably be moved to debugfs (I would like to place it under
> /sys/kernel/debug/hid/$device/ next to rdesc and event).

Yes, that would make sense.

( ... which reminds me to finally to the Documentation/ABI part in sync
with respect to current HID code again ... )

> By the way, I'm wondering why event does not list any of the reports
> coming from my device though as I understand the code it should be doing
> that before my raw_event function gets called...

Sorry, what 'event' do you mean in 'event does not list any of the
reports'?

Thanks,

--
Jiri Kosina
SUSE Labs, Novell Inc.

2010-02-25 15:30:10

by Bruno Prémont

[permalink] [raw]
Subject: Re: [PATCH 1/3] picolcd: driver for PicoLCD HID device

On Thu, 25 February 2010 Jiri Kosina <[email protected]> wrote:
>
> On Thu, 25 Feb 2010, Bruno Prémont wrote:
>
> > For the two sysfs attributes I currently use, the 'reset' one shall
> > probably be moved to debugfs (I would like to place it under
> > /sys/kernel/debug/hid/$device/ next to rdesc and event).
>
> Yes, that would make sense.

So I will move it.

> ( ... which reminds me to finally to the Documentation/ABI part in
> sync with respect to current HID code again ... )
>
> > By the way, I'm wondering why event does not list any of the
> > reports coming from my device though as I understand the code it
> > should be doing that before my raw_event function gets called...
>
> Sorry, what 'event' do you mean in 'event does not list any of the
> reports'?

Sorry, one 's' that got eaten between my brain and mail client. I
mean events file in debugfs.
For USB HID keyboard each key press generates "report dump" under
/sys/kernel/debug/hid/$device/events but for my PicoLCD I don't get
any entry in there. I'm wondering why.

Thanks,
Bruno

2010-02-25 15:35:07

by Rick L. Vinyard, Jr.

[permalink] [raw]
Subject: Re: [PATCH 1/3] picolcd: driver for PicoLCD HID device

Jiri Kosina wrote:
> On Wed, 24 Feb 2010, Rick L. Vinyard, Jr. wrote:
>
>> The key difference is the replacement of spin_lock() with spin_trylock()
>> such that if the non-interrupt code has already obtained the lock, the
>> interrupt will not deadlock but instead take the else path and schedule
>> a
>> framebuffer update at the next interval.
>
> Why is _irqsave() and/or deferred work not enough? The aproach with
> _trylock() seems to be overly complicated for no good reason (I personally
> become very suspicious every time I see code that is using _trylock()).
>

I was concerned about _irqsave() because the lock is split across two
functions to protect the urb after it is handed off to the usb subsystem
with usb_submit_urb(). It's locked in g13_fb_send() and unlocked in the
urb completion callback.

As for deferred work, the g13_fb_send() is the I/O portion of the deferred
framebuffer callback. I was concerned that without a lock one deferred
update could hand the urb off to the usb subsystem and a second could try
to access it before it was handed back to the driver.

In this case the _trylock() would fail and in the else patch we would
defer yet again until the next update cycle.

I took this approach because usb_interrupt_msg() couldn't be used from an
interrupt context, such as a resume hook because eventually down the chain
it does a wait_for_completion_timeout().

It has the added benefit of reusing the urb instead of creating a new one
for each framebuffer sent out, but that wasn't a reason... just a side
effect.

The downside is that I had to manage the urb.

One thing I could do is forget about directly calling g13_fb_send() from
any context and instead use the deferred framebuffer workqueue.

That's probably a simpler approach anyway.

> [ by the way, Rick, are you planning to resubmit the G13 driver with
> incorporated feedback from the last review round? ]
>

Yes. I just wanted to get the details of suspend/resume worked out before
resubmitting.

--

Rick

2010-02-25 17:52:51

by Rick L. Vinyard, Jr.

[permalink] [raw]
Subject: Re: [PATCH 1/3] picolcd: driver for PicoLCD HID device


Bruno Prémont wrote:
> The one for deferredio refresh rate should ideally go below fb device
> (and I guess that might also be of use for other users of deferredio)
>

I'm testing a patch now. I didn't expect anyone else would really need it,
but it does help with throttling bus traffic from userspace.

If it's in fbsysfs.c we can both eliminate it from our drivers.

--

Rick

2010-02-26 08:12:24

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH 1/3] picolcd: driver for PicoLCD HID device

On Thu, Feb 25, 2010 at 08:34:42AM -0700, Rick L. Vinyard, Jr. wrote:
> Jiri Kosina wrote:
> > On Wed, 24 Feb 2010, Rick L. Vinyard, Jr. wrote:
> >
> >> The key difference is the replacement of spin_lock() with spin_trylock()
> >> such that if the non-interrupt code has already obtained the lock, the
> >> interrupt will not deadlock but instead take the else path and schedule
> >> a
> >> framebuffer update at the next interval.
> >
> > Why is _irqsave() and/or deferred work not enough? The aproach with
> > _trylock() seems to be overly complicated for no good reason (I personally
> > become very suspicious every time I see code that is using _trylock()).
> >
>
> I was concerned about _irqsave() because the lock is split across two
> functions to protect the urb after it is handed off to the usb subsystem
> with usb_submit_urb(). It's locked in g13_fb_send() and unlocked in the
> urb completion callback.
>
> As for deferred work, the g13_fb_send() is the I/O portion of the deferred
> framebuffer callback. I was concerned that without a lock one deferred
> update could hand the urb off to the usb subsystem and a second could try
> to access it before it was handed back to the driver.
>
> In this case the _trylock() would fail and in the else patch we would
> defer yet again until the next update cycle.
>

What you are looking for here is called test_and_set_bit(). Do not muddy
the waters with a lock.

--
Dmitry

2010-02-26 08:15:24

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH 1/3] picolcd: driver for PicoLCD HID device

On Thu, Feb 25, 2010 at 12:07:38PM +0100, Jiri Kosina wrote:
> On Wed, 24 Feb 2010, Bruno Pr?mont wrote:
>
> > +/* Update fb_vbitmap from the screen_base and send changed tiles to device */
> > +static void picolcd_fb_update(struct picolcd_data *data)
> > +{
> > + int chip, tile;
> > +
> > + spin_lock(&data->lock);
> > + if (!(data->status & PICOLCD_READY_FB)) {
> > + spin_unlock(&data->lock);
> > + picolcd_fb_reset(data->hdev, 0);
> > + } else
> > + spin_unlock(&data->lock);
>
> Please put the brackets to the else branch as well.
>
Well, it looks like it can be rewritten as:

spin_lock(&data->lock);
need_reset = !(data->status & PICOLCD_READY_FB);
spin_unlock(&data->lock);

if (need_reset)
picolcd_fb_reset(data->hdev, 0);

Which plainly shows that the locking here is bogus.

--
Dmitry

2010-02-26 12:21:46

by Bruno Prémont

[permalink] [raw]
Subject: [PATCH] backlight, panasonic-laptop: fix incomplete registration failure handling

Properly return backlight registration error to parent.
Mark struct backlight_ops as const.

Signed-off-by: Bruno Prémont <[email protected]>
Acked-by: Harald Welte <[email protected]> (registration failure)
---
drivers/platform/x86/panasonic-laptop.c | 4 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/panasonic-laptop.c b/drivers/platform/x86/panasonic-laptop.c
index 9012d8d..9b17343 100644
--- a/drivers/platform/x86/panasonic-laptop.c
+++ b/drivers/platform/x86/panasonic-laptop.c
@@ -352,7 +352,7 @@ static int bl_set_status(struct backlight_device *bd)
return acpi_pcc_write_sset(pcc, SINF_DC_CUR_BRIGHT, bright);
}

-static struct backlight_ops pcc_backlight_ops = {
+static const struct backlight_ops pcc_backlight_ops = {
.get_brightness = bl_get,
.update_status = bl_set_status,
};
@@ -644,8 +644,10 @@ static int acpi_pcc_hotkey_add(struct acpi_device *device)
/* initialize backlight */
pcc->backlight = backlight_device_register("panasonic", NULL, pcc,
&pcc_backlight_ops);
- if (IS_ERR(pcc->backlight))
+ if (IS_ERR(pcc->backlight)) {
+ result = PTR_ERR(pcc->backlight);
goto out_input;
+ }

if (!acpi_pcc_retrieve_biosdata(pcc, pcc->sinf)) {
ACPI_DEBUG_PRINT((ACPI_DB_ERROR,
--
1.6.4.4