From 39a894043ffc2ec60f2cbc743049021d9b3aad62 Mon Sep 17 00:00:00 2001 From: Matt Sealey Date: Mon, 26 Mar 2012 17:34:03 -0500 Subject: [PATCH 01/22] build: workaround for Linaro gcc-4.6 cross compilers --- Makefile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Makefile b/Makefile index e6b7c1cfe4c..f7419f978f4 100644 --- a/Makefile +++ b/Makefile @@ -571,6 +571,9 @@ KBUILD_CFLAGS += $(call cc-option,-fno-strict-overflow) # revert to pre-gcc-4.4 behaviour of .eh_frame KBUILD_CFLAGS += $(call cc-option,-fno-dwarf2-cfi-asm) +# fix a Linaro gcc-4.6 bug (on Ubuntu Precise at least) that stops old ARM kernels booting +KBUILD_CFLAGS += $(call cc-option,-mno-unaligned-access) + # Add user supplied CPPFLAGS, AFLAGS and CFLAGS as the last assignments # But warn user when we do so warn-assign = \ From 89e13e660aa39b71d4737dc9a8ce18feb71547ef Mon Sep 17 00:00:00 2001 From: Matt Sealey Date: Thu, 19 Apr 2012 13:23:30 -0500 Subject: [PATCH 02/22] efikamx: correct IPU version and don't want 600ms for SIIHDMI reset (it doesn't take that long) --- arch/arm/mach-mx5/mx51_efikamx_display.c | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/arch/arm/mach-mx5/mx51_efikamx_display.c b/arch/arm/mach-mx5/mx51_efikamx_display.c index 9795bea8551..2407b75e92c 100644 --- a/arch/arm/mach-mx5/mx51_efikamx_display.c +++ b/arch/arm/mach-mx5/mx51_efikamx_display.c @@ -44,7 +44,7 @@ extern void mx5_ipu_reset(void); static struct mxc_ipu_config mxc_ipu_data = { - .rev = 2, + .rev = 3, .reset = mx5_ipu_reset, }; @@ -74,14 +74,6 @@ void mx51_efikamx_display_reset(void) msleep(1); /* SII9022 Treset >= 100us */ gpio_set_value(IOMUX_TO_GPIO(EFIKAMX_DISPLAY_RESET), 0); - /* - * this is a very long time, but we need to wait this long for the - * SII9022 to come out of reset *AND* for any hotplug events to have - * registered and a sink detection to be absolutely accurate - * (SII9022 programmer's reference p42: - * Tplug_dly min. 400 typ. 480 max. 600ms) - */ - msleep(600); } From 21371b733bf397e4923768e89a8e73f9d7ac2e3a Mon Sep 17 00:00:00 2001 From: Matt Sealey Date: Thu, 19 Apr 2012 13:24:59 -0500 Subject: [PATCH 03/22] siihdmi: add definition of driver name to simplify debug statements (header) --- drivers/video/mxc/siihdmi.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/video/mxc/siihdmi.h b/drivers/video/mxc/siihdmi.h index f5ac036b902..f8b9a0c23b7 100644 --- a/drivers/video/mxc/siihdmi.h +++ b/drivers/video/mxc/siihdmi.h @@ -35,6 +35,8 @@ #include #include +#define SIIHDMI_NAME "siihdmi" + /* TPI registers */ #define SIIHDMI_TPI_REG_VIDEO_MODE_DATA_BASE (0x00) #define SIIHDMI_TPI_REG_PIXEL_CLOCK_LSB (0x00) From 7c070eb30885835de510ea93d305b05f79dac8c9 Mon Sep 17 00:00:00 2001 From: Matt Sealey Date: Thu, 19 Apr 2012 13:37:00 -0500 Subject: [PATCH 04/22] siihdmi: rework debug statements not to redefine DEBUG, and use SIIHDMI_NAME --- drivers/video/mxc/siihdmi.c | 65 +++++++++++++++++++------------------ 1 file changed, 33 insertions(+), 32 deletions(-) diff --git a/drivers/video/mxc/siihdmi.c b/drivers/video/mxc/siihdmi.c index bb3489c8045..89f5a23d330 100644 --- a/drivers/video/mxc/siihdmi.c +++ b/drivers/video/mxc/siihdmi.c @@ -48,11 +48,12 @@ /* logging helpers */ -#define CONTINUE(fmt, ...) printk(KERN_CONT fmt, ## __VA_ARGS__) -#define DEBUG(fmt, ...) printk(KERN_DEBUG "SIIHDMI: " fmt, ## __VA_ARGS__) -#define ERROR(fmt, ...) printk(KERN_ERR "SIIHDMI: " fmt, ## __VA_ARGS__) -#define WARNING(fmt, ...) printk(KERN_WARNING "SIIHDMI: " fmt, ## __VA_ARGS__) -#define INFO(fmt, ...) printk(KERN_INFO "SIIHDMI: " fmt, ## __VA_ARGS__) +#define PR_PREFIX SIIHDMI_NAME ": " +#define CONTINUE(fmt, ...) pr_cont(fmt, ## __VA_ARGS__) +#define DBG(fmt, ...) pr_debug(PR_PREFIX fmt, ## __VA_ARGS__) +#define ERR(fmt, ...) pr_err(PR_PREFIX fmt, ## __VA_ARGS__) +#define WARNING(fmt, ...) pr_warning(PR_PREFIX fmt, ## __VA_ARGS__) +#define INFO(fmt, ...) pr_info(PR_PREFIX fmt, ## __VA_ARGS__) /* module parameters */ @@ -150,7 +151,7 @@ static inline int siihdmi_power_up(struct siihdmi_tx *tx) SIIHDMI_TPI_REG_PWR_STATE, SIIHDMI_POWER_STATE_D0); if (ret < 0) - ERROR("unable to power up transmitter\n"); + ERR("unable to power up transmitter\n"); return ret; } @@ -172,7 +173,7 @@ static inline int siihdmi_power_down(struct siihdmi_tx *tx) ret = i2c_smbus_write_byte_data(tx->client, SIIHDMI_TPI_REG_SYS_CTRL, ctrl); if (ret < 0) { - ERROR("unable to power down transmitter\n"); + ERR("unable to power down transmitter\n"); return ret; } @@ -180,7 +181,7 @@ static inline int siihdmi_power_down(struct siihdmi_tx *tx) SIIHDMI_TPI_REG_PWR_STATE, SIIHDMI_POWER_STATE_D2); if (ret < 0) { - ERROR("unable to set transmitter into D2\n"); + ERR("unable to set transmitter into D2\n"); return ret; } @@ -203,7 +204,7 @@ static int siihdmi_initialise(struct siihdmi_tx *tx) /* step 2: detect revision */ if ((ret = siihdmi_detect_revision(tx)) < 0) { - DEBUG("unable to detect device revision\n"); + DBG("unable to detect device revision\n"); return ret; } @@ -245,13 +246,13 @@ static inline void _process_cea861_vsdb(struct siihdmi_tx *tx, max_tmds = KHZ2PICOS(vsdb->max_tmds_clock * 200); - DEBUG("HDMI VSDB detected (basic audio %ssupported)\n", + DBG("HDMI VSDB detected (basic audio %ssupported)\n", tx->audio.available ? "" : "not "); if (!forcedvi) { tx->sink.type = SINK_TYPE_HDMI; } else { - DEBUG("Sink type forced to DVI despite VSDB\n"); + DBG("Sink type forced to DVI despite VSDB\n"); tx->sink.type = SINK_TYPE_DVI; } @@ -281,7 +282,7 @@ static inline void _process_cea861_video(struct siihdmi_tx *tx, } } - DEBUG("%u modes parsed from CEA video data block\n", count); + DBG("%u modes parsed from CEA video data block\n", count); } static inline void _process_cea861_extended(struct siihdmi_tx *tx, @@ -391,7 +392,7 @@ static void siihdmi_set_vmode_registers(struct siihdmi_tx *tx, sizeof(vmode), (u8 *) vmode); if (ret < 0) - DEBUG("unable to write video mode data\n"); + DBG("unable to write video mode data\n"); /* input format */ format = SIIHDMI_INPUT_COLOR_SPACE_RGB @@ -402,7 +403,7 @@ static void siihdmi_set_vmode_registers(struct siihdmi_tx *tx, SIIHDMI_TPI_REG_AVI_INPUT_FORMAT, format); if (ret < 0) - DEBUG("unable to set input format\n"); + DBG("unable to set input format\n"); /* output format */ format = SIIHDMI_OUTPUT_VIDEO_RANGE_COMPRESSION_AUTO @@ -418,7 +419,7 @@ static void siihdmi_set_vmode_registers(struct siihdmi_tx *tx, SIIHDMI_TPI_REG_AVI_OUTPUT_FORMAT, format); if (ret < 0) - DEBUG("unable to set output format\n"); + DBG("unable to set output format\n"); } static int siihdmi_clear_avi_info_frame(struct siihdmi_tx *tx) @@ -432,7 +433,7 @@ static int siihdmi_clear_avi_info_frame(struct siihdmi_tx *tx) SIIHDMI_TPI_REG_AVI_INFO_FRAME_BASE, sizeof(buffer), buffer); if (ret < 0) - DEBUG("unable to clear avi info frame\n"); + DBG("unable to clear avi info frame\n"); return ret; } @@ -458,7 +459,7 @@ static int siihdmi_set_avi_info_frame(struct siihdmi_tx *tx, int vic) BUG_ON(tx->sink.type != SINK_TYPE_HDMI); - DEBUG("AVI InfoFrame sending Video Format %d\n", vic); + DBG("AVI InfoFrame sending Video Format %d\n", vic); switch (tx->sink.scanning) { case SCANNING_UNDERSCANNED: @@ -480,7 +481,7 @@ static int siihdmi_set_avi_info_frame(struct siihdmi_tx *tx, int vic) sizeof(avi) - SIIHDMI_AVI_INFO_FRAME_OFFSET, ((u8 *) &avi) + SIIHDMI_AVI_INFO_FRAME_OFFSET); if (ret < 0) - DEBUG("unable to write avi info frame\n"); + DBG("unable to write avi info frame\n"); return ret; } @@ -525,7 +526,7 @@ static int siihdmi_set_audio_info_frame(struct siihdmi_tx *tx) sizeof(packet), (u8 *) &packet); if (ret < 0) - DEBUG("unable to write audio info frame\n"); + DBG("unable to write audio info frame\n"); return ret; } @@ -567,7 +568,7 @@ static int siihdmi_set_spd_info_frame(struct siihdmi_tx *tx) sizeof(packet), (u8 *) &packet); if (ret < 0) - DEBUG("unable to write SPD info frame\n"); + DBG("unable to write SPD info frame\n"); return ret; } @@ -714,7 +715,7 @@ static int siihdmi_set_resolution(struct siihdmi_tx *tx, SIIHDMI_TPI_REG_SYS_CTRL, ctrl); if (ret < 0) - DEBUG("unable to AV Mute!\n"); + DBG("unable to AV Mute!\n"); } msleep(SIIHDMI_CTRL_INFO_FRAME_DRAIN_TIME); } @@ -725,7 +726,7 @@ static int siihdmi_set_resolution(struct siihdmi_tx *tx, SIIHDMI_TPI_REG_SYS_CTRL, ctrl); if (ret < 0) - DEBUG("unable to prepare for resolution change\n"); + DBG("unable to prepare for resolution change\n"); /* * step 3: change video resolution @@ -761,7 +762,7 @@ static int siihdmi_set_resolution(struct siihdmi_tx *tx, SIIHDMI_TPI_REG_SYS_CTRL, ctrl); if (ret < 0) - DEBUG("unable to enable the display\n"); + DBG("unable to enable the display\n"); /* step 8: (optionally) un-blank the display */ if (tx->sink.type == SINK_TYPE_HDMI && useavmute) { @@ -770,7 +771,7 @@ static int siihdmi_set_resolution(struct siihdmi_tx *tx, SIIHDMI_TPI_REG_SYS_CTRL, ctrl); if (ret < 0) - DEBUG("unable to unmute the display\n"); + DBG("unable to unmute the display\n"); } /* step 9: (potentially) enable HDCP */ @@ -1031,7 +1032,7 @@ static inline int siihdmi_read_edid(struct siihdmi_tx *tx, u8 *edid, int size) ret = i2c_transfer(tx->client->adapter, request, ARRAY_SIZE(request)); if (ret != ARRAY_SIZE(request)) - DEBUG("unable to read EDID block\n"); + DBG("unable to read EDID block\n"); return ret; } @@ -1053,7 +1054,7 @@ static int siihdmi_detect_monitor(struct siihdmi_tx *tx) SIIHDMI_TPI_REG_SYS_CTRL, ctrl | SIIHDMI_SYS_CTRL_DDC_BUS_REQUEST); if (ret < 0) { - DEBUG("unable to request DDC bus\n"); + DBG("unable to request DDC bus\n"); return ret; } @@ -1074,7 +1075,7 @@ static int siihdmi_detect_monitor(struct siihdmi_tx *tx) SIIHDMI_SYS_CTRL_DDC_BUS_REQUEST | SIIHDMI_SYS_CTRL_DDC_BUS_OWNER_HOST); if (ret < 0) { - DEBUG("unable to take ownership of the DDC bus\n"); + DBG("unable to take ownership of the DDC bus\n"); goto relinquish; } @@ -1185,7 +1186,7 @@ static int siihdmi_setup_display(struct siihdmi_tx *tx) tx->audio.available = false; isr = i2c_smbus_read_byte_data(tx->client, SIIHDMI_TPI_REG_ISR); - DEBUG("hotplug: display %s, powered %s\n", + DBG("hotplug: display %s, powered %s\n", (isr & SIIHDMI_ISR_DISPLAY_ATTACHED) ? "attached" : "detached", (isr & SIIHDMI_ISR_RECEIVER_SENSE) ? "on" : "off"); @@ -1203,14 +1204,14 @@ static int siihdmi_setup_display(struct siihdmi_tx *tx) if (sysfs_create_link_nowarn(&tx->info->dev->kobj, &tx->client->dev.kobj, "phys-link") < 0) - ERROR("failed to create device symlink"); + ERR("failed to create device symlink"); break; } } if (tx->info == NULL) { - ERROR("unable to find video framebuffer\n"); + ERR("unable to find video framebuffer\n"); return -1; } @@ -1292,7 +1293,7 @@ static int siihdmi_fb_event_handler(struct notifier_block *nb, } break; default: - DEBUG("unhandled fb event 0x%lx", val); + DBG("unhandled fb event 0x%lx", val); break; } @@ -1323,7 +1324,7 @@ static void siihdmi_hotplug_event(struct work_struct *work) if (~isr & SIIHDMI_ISR_HOT_PLUG_EVENT) goto complete; - DEBUG("hotplug: display %s, powered %s\n", + DBG("hotplug: display %s, powered %s\n", (isr & SIIHDMI_ISR_DISPLAY_ATTACHED) ? "attached" : "detached", (isr & SIIHDMI_ISR_RECEIVER_SENSE) ? "on" : "off"); From a39558634ad1c9954e91573fcce724d59b1dca55 Mon Sep 17 00:00:00 2001 From: Matt Sealey Date: Thu, 19 Apr 2012 13:39:15 -0500 Subject: [PATCH 05/22] siihdmi: use SIIHDMI_NAME for driver data too --- drivers/video/mxc/siihdmi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/video/mxc/siihdmi.c b/drivers/video/mxc/siihdmi.c index 89f5a23d330..066039bf591 100644 --- a/drivers/video/mxc/siihdmi.c +++ b/drivers/video/mxc/siihdmi.c @@ -1482,12 +1482,12 @@ static int __devexit siihdmi_remove(struct i2c_client *client) } static const struct i2c_device_id siihdmi_device_table[] = { - { "siihdmi", 0 }, + { SIIHDMI_NAME, 0 }, { }, }; static struct i2c_driver siihdmi_driver = { - .driver = { .name = "siihdmi" }, + .driver = { .name = SIIHDMI_NAME }, .probe = siihdmi_probe, .remove = siihdmi_remove, .id_table = siihdmi_device_table, From 1327336167f87e57f871fb1abc8786c120ce89b6 Mon Sep 17 00:00:00 2001 From: Matt Sealey Date: Thu, 19 Apr 2012 13:42:23 -0500 Subject: [PATCH 06/22] siihdmi: correct debug not to show display "power" on or off but more directly refer to "receiver sense" active or inactive. Also distinguish between the debug in the hotplug service loop and the almost identical output in the display setup code --- drivers/video/mxc/siihdmi.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/video/mxc/siihdmi.c b/drivers/video/mxc/siihdmi.c index 066039bf591..1dec7a084a2 100644 --- a/drivers/video/mxc/siihdmi.c +++ b/drivers/video/mxc/siihdmi.c @@ -1186,9 +1186,9 @@ static int siihdmi_setup_display(struct siihdmi_tx *tx) tx->audio.available = false; isr = i2c_smbus_read_byte_data(tx->client, SIIHDMI_TPI_REG_ISR); - DBG("hotplug: display %s, powered %s\n", + DBG("setup: display %s, receiver sense %s\n", (isr & SIIHDMI_ISR_DISPLAY_ATTACHED) ? "attached" : "detached", - (isr & SIIHDMI_ISR_RECEIVER_SENSE) ? "on" : "off"); + (isr & SIIHDMI_ISR_RECEIVER_SENSE) ? "active" : "inactive"); if (~isr & SIIHDMI_ISR_DISPLAY_ATTACHED) return siihdmi_power_down(tx); @@ -1324,9 +1324,9 @@ static void siihdmi_hotplug_event(struct work_struct *work) if (~isr & SIIHDMI_ISR_HOT_PLUG_EVENT) goto complete; - DBG("hotplug: display %s, powered %s\n", + DBG("hotplug: display %s, receiver sense %s\n", (isr & SIIHDMI_ISR_DISPLAY_ATTACHED) ? "attached" : "detached", - (isr & SIIHDMI_ISR_RECEIVER_SENSE) ? "on" : "off"); + (isr & SIIHDMI_ISR_RECEIVER_SENSE) ? "active" : "inactive"); if (isr & SIIHDMI_ISR_HOT_PLUG_EVENT) { if (isr & SIIHDMI_ISR_DISPLAY_ATTACHED) From badb6863298ca1f7788dda63a4a90cc0570b71f2 Mon Sep 17 00:00:00 2001 From: Matt Sealey Date: Thu, 19 Apr 2012 13:46:11 -0500 Subject: [PATCH 07/22] siihdmi: only mark insufficient margin in the mode list for 800x600 as all other modes tested actually seem to work okay. --- drivers/video/mxc/siihdmi.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/video/mxc/siihdmi.c b/drivers/video/mxc/siihdmi.c index 1dec7a084a2..ed58978d29c 100644 --- a/drivers/video/mxc/siihdmi.c +++ b/drivers/video/mxc/siihdmi.c @@ -849,12 +849,16 @@ static void siihdmi_sanitize_modelist(struct siihdmi_tx * const tx) reason = "doublescan"; } else if (mode->pixclock < tx->platform->pixclock) { reason = "pixel clock exceeded"; - } else if ((tx->sink.type == SINK_TYPE_HDMI) && mode->lower_margin < 2) { + } else if ((tx->sink.type == SINK_TYPE_HDMI) + && (mode->lower_margin < 2) + && (mode->xres == 800 && mode->yres == 600)) { /* * HDMI spec (§ 5.1.2) stipulates ≥2 lines of vsync * * We do not care so much on DVI, although it may be that the SII9022 cannot - * actually display this mode. Requires testing!! + * actually display this mode. In any case it only seems to make a difference + * on the VESA (actually old VGA timing) 800x600 mode common on some 7" panels + * and not-so-modern monitors. */ reason = "insufficient margin"; } else { From 2bba60824fd6f146e38cd2978fa9b8a8238762f5 Mon Sep 17 00:00:00 2001 From: Matt Sealey Date: Thu, 19 Apr 2012 13:49:00 -0500 Subject: [PATCH 08/22] siihdmi: correct bus timeouts to convert ms to jiffies for the time_after function --- drivers/video/mxc/siihdmi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/video/mxc/siihdmi.c b/drivers/video/mxc/siihdmi.c index ed58978d29c..f8bb0be6404 100644 --- a/drivers/video/mxc/siihdmi.c +++ b/drivers/video/mxc/siihdmi.c @@ -1068,7 +1068,7 @@ static int siihdmi_detect_monitor(struct siihdmi_tx *tx) ctrl = i2c_smbus_read_byte_data(tx->client, SIIHDMI_TPI_REG_SYS_CTRL); } while ((~ctrl & SIIHDMI_SYS_CTRL_DDC_BUS_GRANTED) && - !time_after(jiffies, start + bus_timeout)); + !time_after(jiffies, start + msecs_to_jiffies(bus_timeout))); if (~ctrl & SIIHDMI_SYS_CTRL_DDC_BUS_GRANTED) goto relinquish; @@ -1159,7 +1159,7 @@ relinquish: ctrl = i2c_smbus_read_byte_data(tx->client, SIIHDMI_TPI_REG_SYS_CTRL); } while ((ctrl & SIIHDMI_SYS_CTRL_DDC_BUS_GRANTED) && - !time_after(jiffies, start + bus_timeout)); + !time_after(jiffies, start + msecs_to_jiffies(bus_timeout))); /* now, force the operational mode (HDMI or DVI) based on sink * type and make it stick with a power up request (pg 27) From d78c91e816e2f8c760bf53563d9351aae3840290 Mon Sep 17 00:00:00 2001 From: Matt Sealey Date: Thu, 19 Apr 2012 13:49:56 -0500 Subject: [PATCH 09/22] siihdmi: do the same msecs_to_jiffies for the identify timeout to match the DDC timeout --- drivers/video/mxc/siihdmi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/video/mxc/siihdmi.c b/drivers/video/mxc/siihdmi.c index f8bb0be6404..57390d7310c 100644 --- a/drivers/video/mxc/siihdmi.c +++ b/drivers/video/mxc/siihdmi.c @@ -110,7 +110,7 @@ static int siihdmi_detect_revision(struct siihdmi_tx *tx) data = i2c_smbus_read_byte_data(tx->client, SIIHDMI_TPI_REG_DEVICE_ID); } while (data != SIIHDMI_DEVICE_ID_902x && - !time_after(jiffies, start + bus_timeout)); + !time_after(jiffies, start + msecs_to_jiffies(bus_timeout))); if (data != SIIHDMI_DEVICE_ID_902x) return -ENODEV; From 37c5293a2e665befa27db8f8ccee56d0042ca840 Mon Sep 17 00:00:00 2001 From: Matt Sealey Date: Thu, 19 Apr 2012 13:52:43 -0500 Subject: [PATCH 10/22] siihdmi: comment about setting up interrupt service --- drivers/video/mxc/siihdmi.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/video/mxc/siihdmi.c b/drivers/video/mxc/siihdmi.c index 57390d7310c..abbdf09b12a 100644 --- a/drivers/video/mxc/siihdmi.c +++ b/drivers/video/mxc/siihdmi.c @@ -222,7 +222,14 @@ static int siihdmi_initialise(struct siihdmi_tx *tx) /* step 8: configure embedded sync extraction */ - /* step 9: setup interrupt service */ + /* step 9: setup interrupt service + * + * (SII9022 programmer's reference p42: + * Tplug_dly min. 400 typ. 480 max. 600ms) + * do we need to wait here, or do we know this has already + * happened in hardware? + */ + if (tx->hotplug.enabled) { ret = i2c_smbus_write_byte_data(tx->client, SIIHDMI_TPI_REG_IER, From ddc6441e1379329c2e6e14057c5aa5c88a01c0a0 Mon Sep 17 00:00:00 2001 From: Matt Sealey Date: Thu, 19 Apr 2012 13:55:52 -0500 Subject: [PATCH 11/22] siihdmi: fix a division by zero when HDMI VSDB gives out 0 as the maximum TMDS clock capability --- drivers/video/mxc/siihdmi.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/video/mxc/siihdmi.c b/drivers/video/mxc/siihdmi.c index abbdf09b12a..11aa4835249 100644 --- a/drivers/video/mxc/siihdmi.c +++ b/drivers/video/mxc/siihdmi.c @@ -245,13 +245,16 @@ static int siihdmi_initialise(struct siihdmi_tx *tx) static inline void _process_cea861_vsdb(struct siihdmi_tx *tx, const struct hdmi_vsdb * const vsdb) { - unsigned int max_tmds; + unsigned int max_tmds = 0; if (memcmp(vsdb->ieee_registration, CEA861_OUI_REGISTRATION_ID_HDMI_LSB, - sizeof(vsdb->ieee_registration))) + sizeof(vsdb->ieee_registration))) { + WARNING("VSDB does not contain HDMI OUI\n"); return; + } - max_tmds = KHZ2PICOS(vsdb->max_tmds_clock * 200); + if (vsdb->max_tmds_clock) + max_tmds = KHZ2PICOS(vsdb->max_tmds_clock * 200); DBG("HDMI VSDB detected (basic audio %ssupported)\n", tx->audio.available ? "" : "not "); From 37cd50e9ff059360da2d8d09b93a2a5618ad95d7 Mon Sep 17 00:00:00 2001 From: Matt Sealey Date: Thu, 19 Apr 2012 13:59:01 -0500 Subject: [PATCH 12/22] siihdmi: strncmp against modedb instead of structure memcmp to save time and effort matching CEA VIC --- drivers/video/mxc/siihdmi.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/video/mxc/siihdmi.c b/drivers/video/mxc/siihdmi.c index 11aa4835249..3dd24004e32 100644 --- a/drivers/video/mxc/siihdmi.c +++ b/drivers/video/mxc/siihdmi.c @@ -678,7 +678,11 @@ static int siihdmi_find_vic_from_modedb(const struct fb_videomode *mode) for (vic = 1; vic <= 64; vic++) { - if (!memcmp((void *)&cea_modes[vic], (void *)mode, sizeof(struct fb_videomode))) + /* for comment's sake, "CEA VIC nn" is 10 characters + * watch out porting this as it relies the CEA VIC nn string + * to be in the modedb + */ + if (!strncmp(cea_modes[vic].name, mode->name, 10)) return vic; } return 0; From 9960ff68e1f42b127943cee4f97743d76ba31f32 Mon Sep 17 00:00:00 2001 From: Matt Sealey Date: Thu, 19 Apr 2012 14:01:42 -0500 Subject: [PATCH 13/22] siihdmi: clarify power modes debug --- drivers/video/mxc/siihdmi.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/video/mxc/siihdmi.c b/drivers/video/mxc/siihdmi.c index 3dd24004e32..236f0fe8a8e 100644 --- a/drivers/video/mxc/siihdmi.c +++ b/drivers/video/mxc/siihdmi.c @@ -152,6 +152,8 @@ static inline int siihdmi_power_up(struct siihdmi_tx *tx) SIIHDMI_POWER_STATE_D0); if (ret < 0) ERR("unable to power up transmitter\n"); + else + WARNING("powered to D0\n"); return ret; } @@ -183,7 +185,8 @@ static inline int siihdmi_power_down(struct siihdmi_tx *tx) if (ret < 0) { ERR("unable to set transmitter into D2\n"); return ret; - } + } else + WARNING("powered to D2\n"); return 0; } From 64cf5e0b27369e25ff6a5bd15c59e0422c0f4a45 Mon Sep 17 00:00:00 2001 From: Matt Sealey Date: Thu, 19 Apr 2012 14:03:47 -0500 Subject: [PATCH 14/22] siihdmi: downgrade to 640x480 output while getting EDID to match HDMI specification --- drivers/video/mxc/siihdmi.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/drivers/video/mxc/siihdmi.c b/drivers/video/mxc/siihdmi.c index 236f0fe8a8e..75658b75b62 100644 --- a/drivers/video/mxc/siihdmi.c +++ b/drivers/video/mxc/siihdmi.c @@ -1236,6 +1236,15 @@ static int siihdmi_setup_display(struct siihdmi_tx *tx) return -1; } + /* + * HDMI spec says we should use standard 640x480 while getting + * the EDID - this helps a lot even if we don't care about the + * standard as many dual/multi-input monitors run a round of + * autodetection to find the active display port, which causes + * some havoc with the hotplug handler + */ + siihdmi_set_resolution(tx, &vesa_modes[3]); + /* use EDID to detect sink characteristics */ ret = siihdmi_detect_monitor(tx); if (ret < 0) From c00322177f9468ee267bd7dfe48ab59bf887a7f2 Mon Sep 17 00:00:00 2001 From: Matt Sealey Date: Thu, 19 Apr 2012 14:43:34 -0500 Subject: [PATCH 15/22] siihdmi: fix some checkpatch errors and realign some stuff to reduce warnings. Now we're only guilty of going over 80 columns, but Camelot is a silly place. --- drivers/video/mxc/siihdmi.c | 101 ++++++++++++++++++++---------------- 1 file changed, 56 insertions(+), 45 deletions(-) diff --git a/drivers/video/mxc/siihdmi.c b/drivers/video/mxc/siihdmi.c index 75658b75b62..8d91ffadcad 100644 --- a/drivers/video/mxc/siihdmi.c +++ b/drivers/video/mxc/siihdmi.c @@ -65,38 +65,46 @@ static unsigned int seventwenty = 1; module_param(seventwenty, uint, 0644); MODULE_PARM_DESC(seventwenty, "attempt to use 720p mode"); -static unsigned int teneighty = 0; +static unsigned int teneighty; module_param(teneighty, uint, 0644); MODULE_PARM_DESC(teneighty, "attempt to use 1080p mode"); static unsigned int useitmodes = 1; module_param(useitmodes, uint, 0644); -MODULE_PARM_DESC(useitmodes, "prefer IT modes over CEA modes when sanitizing the modelist"); +MODULE_PARM_DESC(useitmodes, + "prefer IT modes over CEA modes when sanitizing the modelist"); -static unsigned int modevic = 0; +static unsigned int modevic; module_param_named(vic, modevic, uint, 0644); -MODULE_PARM_DESC(modevic, "CEA VIC to try and match before autodetection"); +MODULE_PARM_DESC(modevic, + "CEA VIC to try and match before autodetection"); -static unsigned int forcedvi = 0; +static unsigned int forcedvi; module_param_named(dvi, forcedvi, uint, 0644); MODULE_PARM_DESC(forcedvi, "Force DVI sink mode"); -static unsigned int useavmute = 0; +static unsigned int useavmute; module_param(useavmute, uint, 0644); MODULE_PARM_DESC(useavmute, "perform HDMI AV Mute when blanking screen"); static int siihdmi_read_internal(struct siihdmi_tx *tx, u8 page, u8 offset) { - i2c_smbus_write_byte_data(tx->client, SIIHDMI_INTERNAL_REG_SET_PAGE, page); - i2c_smbus_write_byte_data(tx->client, SIIHDMI_INTERNAL_REG_SET_OFFSET, offset); - return i2c_smbus_read_byte_data(tx->client, SIIHDMI_INTERNAL_REG_ACCESS); + i2c_smbus_write_byte_data(tx->client, + SIIHDMI_INTERNAL_REG_SET_PAGE, page); + i2c_smbus_write_byte_data(tx->client, + SIIHDMI_INTERNAL_REG_SET_OFFSET, offset); + return i2c_smbus_read_byte_data(tx->client, + SIIHDMI_INTERNAL_REG_ACCESS); } static void siihdmi_write_internal(struct siihdmi_tx *tx, u8 page, u8 offset, u8 value) { - i2c_smbus_write_byte_data(tx->client, SIIHDMI_INTERNAL_REG_SET_PAGE, page); - i2c_smbus_write_byte_data(tx->client, SIIHDMI_INTERNAL_REG_SET_OFFSET, offset); - i2c_smbus_write_byte_data(tx->client, SIIHDMI_INTERNAL_REG_ACCESS, value); + i2c_smbus_write_byte_data(tx->client, + SIIHDMI_INTERNAL_REG_SET_PAGE, page); + i2c_smbus_write_byte_data(tx->client, + SIIHDMI_INTERNAL_REG_SET_OFFSET, offset); + i2c_smbus_write_byte_data(tx->client, + SIIHDMI_INTERNAL_REG_ACCESS, value); } @@ -206,13 +214,15 @@ static int siihdmi_initialise(struct siihdmi_tx *tx) } /* step 2: detect revision */ - if ((ret = siihdmi_detect_revision(tx)) < 0) { + ret = siihdmi_detect_revision(tx); + if (ret < 0) { DBG("unable to detect device revision\n"); return ret; } /* step 3: power up transmitter */ - if ((ret = siihdmi_power_up(tx)) < 0) + ret = siihdmi_power_up(tx); + if (ret < 0) return ret; /* step 4: configure input bus and pixel repetition */ @@ -679,8 +689,7 @@ static int siihdmi_find_vic_from_modedb(const struct fb_videomode *mode) { int vic; - for (vic = 1; vic <= 64; vic++) - { + for (vic = 1; vic <= 64; vic++) { /* for comment's sake, "CEA VIC nn" is 10 characters * watch out porting this as it relies the CEA VIC nn string * to be in the modedb @@ -699,8 +708,8 @@ static int siihdmi_set_resolution(struct siihdmi_tx *tx, u8 ctrl; int ret; - if (0 == memcmp((void *) &tx->sink.current_mode, (void *) mode, sizeof(struct fb_videomode))) - { + if (0 == memcmp((void *) &tx->sink.current_mode, + (void *) mode, sizeof(struct fb_videomode))) { return 0; } @@ -852,10 +861,10 @@ static void siihdmi_sanitize_modelist(struct siihdmi_tx * const tx) const struct fb_videomode *mode; int num_removed = 0; - if ((mode = fb_find_best_display(&tx->info->monspecs, modelist))) + mode = fb_find_best_display(&tx->info->monspecs, modelist); + if (mode) tx->sink.preferred_mode = *mode; - list_for_each_entry_safe(entry, next, modelist, list) { const char *reason = NULL; mode = &entry->mode; @@ -928,9 +937,8 @@ static void siihdmi_sanitize_modelist(struct siihdmi_tx * const tx) struct fb_modelist *modelist = container_of(mode, struct fb_modelist, mode); - if (num_removed == 0) { // first time only + if (num_removed == 0) /* first time only */ INFO("Unsupported modelines:\n"); - } siihdmi_print_modeline(tx, mode, reason); @@ -940,9 +948,8 @@ static void siihdmi_sanitize_modelist(struct siihdmi_tx * const tx) } } - if (num_removed > 0) { + if (num_removed > 0) INFO("discarded %u incompatible modes\n", num_removed); - } } static inline const struct fb_videomode *_match(const struct fb_videomode * const mode, @@ -950,7 +957,8 @@ static inline const struct fb_videomode *_match(const struct fb_videomode * cons { const struct fb_videomode *match; - if ((match = fb_find_best_mode_at_most(mode, modelist))) + match = fb_find_best_mode_at_most(mode, modelist); + if (match) return match; return fb_find_nearest_mode(mode, modelist); @@ -994,8 +1002,10 @@ static const struct fb_videomode *siihdmi_select_video_mode(const struct siihdmi */ if (teneighty) { - if ((tx->sink.preferred_mode.xres == 1680 && tx->sink.preferred_mode.yres == 1050) || - (tx->sink.preferred_mode.xres == 1440 && tx->sink.preferred_mode.yres == 900)) { + if ((tx->sink.preferred_mode.xres == 1680 + && tx->sink.preferred_mode.yres == 1050) || + (tx->sink.preferred_mode.xres == 1440 + && tx->sink.preferred_mode.yres == 900)) { mode = _match(&tx->sink.preferred_mode, &tx->info->modelist); if (mode && (mode->xres == tx->sink.preferred_mode.xres) && (mode->yres == tx->sink.preferred_mode.yres)) @@ -1013,7 +1023,7 @@ static const struct fb_videomode *siihdmi_select_video_mode(const struct siihdmi tx->sink.preferred_mode.xres == 1024 || tx->sink.preferred_mode.xres == 1280) && (tx->sink.preferred_mode.yres == 768 || - tx->sink.preferred_mode.yres == 800) ) { + tx->sink.preferred_mode.yres == 800)) { mode = _match(&tx->sink.preferred_mode, &tx->info->modelist); if (mode && (mode->xres == tx->sink.preferred_mode.xres) && (mode->yres == tx->sink.preferred_mode.yres)) @@ -1027,10 +1037,11 @@ static const struct fb_videomode *siihdmi_select_video_mode(const struct siihdmi /* If we disabled or couldn't find a reasonable mode above, just look for and use the * closest to the monitor preferred mode - we don't care if it is not exact */ - if (tx->sink.preferred_mode.xres && tx->sink.preferred_mode.yres) - if ((mode = _match(&tx->sink.preferred_mode, &tx->info->modelist))) + if (tx->sink.preferred_mode.xres && tx->sink.preferred_mode.yres) { + mode = _match(&tx->sink.preferred_mode, &tx->info->modelist); + if (mode) return mode; - + } /* if no matching mode was found, push 640x480@60 */ INFO("unable to select a suitable video mode, using CEA Mode 1 (640x480@60)\n"); return &cea_modes[1]; @@ -1102,8 +1113,7 @@ static int siihdmi_detect_monitor(struct siihdmi_tx *tx) /* step 5: read edid */ if (tx->edid.length < EDID_BLOCK_SIZE) { - if (tx->edid.data) - kfree(tx->edid.data); + kfree(tx->edid.data); tx->edid.data = kzalloc(EDID_BLOCK_SIZE, GFP_KERNEL); if (!tx->edid.data) { @@ -1251,7 +1261,8 @@ static int siihdmi_setup_display(struct siihdmi_tx *tx) return ret; mode = siihdmi_select_video_mode(tx); - if ((ret = siihdmi_set_resolution(tx, mode)) < 0) + ret = siihdmi_set_resolution(tx, mode); + if (ret < 0) return ret; /* activate the framebuffer */ @@ -1312,14 +1323,14 @@ static int siihdmi_fb_event_handler(struct notifier_block *nb, break; case FB_EVENT_BLANK: switch (*((int *) event->data)) { - case FB_BLANK_POWERDOWN: - /* do NOT siihdmi_power_down() here */ - case FB_BLANK_VSYNC_SUSPEND: - case FB_BLANK_HSYNC_SUSPEND: - case FB_BLANK_NORMAL: - return siihdmi_blank(tx); - case FB_BLANK_UNBLANK: - return siihdmi_unblank(tx); + case FB_BLANK_POWERDOWN: + /* do NOT siihdmi_power_down() here */ + case FB_BLANK_VSYNC_SUSPEND: + case FB_BLANK_HSYNC_SUSPEND: + case FB_BLANK_NORMAL: + return siihdmi_blank(tx); + case FB_BLANK_UNBLANK: + return siihdmi_unblank(tx); } break; default: @@ -1442,7 +1453,8 @@ static int __devinit siihdmi_probe(struct i2c_client *client, tx->hotplug.enabled = true; /* initialise the device */ - if ((ret = siihdmi_initialise(tx)) < 0) + ret = siihdmi_initialise(tx); + if (ret < 0) goto error; ret = siihdmi_setup_display(tx); @@ -1499,8 +1511,7 @@ static int __devexit siihdmi_remove(struct i2c_client *client) sysfs_remove_bin_file(&tx->client->dev.kobj, &tx->audio.attributes); #endif - if (tx->edid.data) - kfree(tx->edid.data); + kfree(tx->edid.data); fb_unregister_client(&tx->nb); siihdmi_power_down(tx); From af99f78e829d42cfe544a1ab65a55dff351c70d0 Mon Sep 17 00:00:00 2001 From: Matt Sealey Date: Thu, 19 Apr 2012 15:02:30 -0500 Subject: [PATCH 16/22] siihdmi: updated checkpatch run, fix a few other things including a serious memset argument order problem (argh!) scripts/checkpatch.pl -f drivers/video/mxc/siihdmi.c --ignore=LONG_LINE,C99_COMMENTS total: 0 errors, 0 warnings, 1555 lines checked NOTE: Ignored message types: C99_COMMENTS LONG_LINE drivers/video/mxc/siihdmi.c has no obvious style problems and is ready for submission. --- drivers/video/mxc/siihdmi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/video/mxc/siihdmi.c b/drivers/video/mxc/siihdmi.c index 8d91ffadcad..1522c549f16 100644 --- a/drivers/video/mxc/siihdmi.c +++ b/drivers/video/mxc/siihdmi.c @@ -715,7 +715,7 @@ static int siihdmi_set_resolution(struct siihdmi_tx *tx, memset((void *) &tx->sink.current_mode, 0, sizeof(struct fb_videomode)); - INFO("selected configuration: \n"); + INFO("selected configuration:\n"); siihdmi_print_modeline(tx, mode, NULL); ctrl = i2c_smbus_read_byte_data(tx->client, SIIHDMI_TPI_REG_SYS_CTRL); @@ -1122,7 +1122,7 @@ static int siihdmi_detect_monitor(struct siihdmi_tx *tx) } tx->edid.length = EDID_BLOCK_SIZE; } else { - memset(tx->edid.data, tx->edid.length, 0); + memset(tx->edid.data, 0, tx->edid.length); } ret = siihdmi_read_edid(tx, tx->edid.data, EDID_BLOCK_SIZE); From 15c4470a5a83c2ce6a63f84b3da414e1e19daf45 Mon Sep 17 00:00:00 2001 From: Matt Sealey Date: Thu, 19 Apr 2012 15:03:09 -0500 Subject: [PATCH 17/22] checkpatch: egregious bump to the version from kernel 3.4rc1 --- scripts/checkpatch.pl | 1471 +++++++++++++++++++++++++++++++++-------- 1 file changed, 1204 insertions(+), 267 deletions(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 2d5ece798c4..de639eeeed5 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -1,8 +1,8 @@ #!/usr/bin/perl -w -# (c) 2001, Dave Jones. (the file handling bit) +# (c) 2001, Dave Jones. (the file handling bit) # (c) 2005, Joel Schopp (the ugly bit) # (c) 2007,2008, Andy Whitcroft (new conditions, test suite) -# (c) 2008, Andy Whitcroft +# (c) 2008-2010 Andy Whitcroft # Licensed under the terms of the GNU GPL License version 2 use strict; @@ -10,7 +10,7 @@ use strict; my $P = $0; $P =~ s@.*/@@g; -my $V = '0.28'; +my $V = '0.32'; use Getopt::Long qw(:config no_auto_abbrev); @@ -26,8 +26,75 @@ my $check = 0; my $summary = 1; my $mailback = 0; my $summary_file = 0; +my $show_types = 0; my $root; my %debug; +my %ignore_type = (); +my @ignore = (); +my $help = 0; +my $configuration_file = ".checkpatch.conf"; + +sub help { + my ($exitcode) = @_; + + print << "EOM"; +Usage: $P [OPTION]... [FILE]... +Version: $V + +Options: + -q, --quiet quiet + --no-tree run without a kernel tree + --no-signoff do not check for 'Signed-off-by' line + --patch treat FILE as patchfile (default) + --emacs emacs compile window format + --terse one line per report + -f, --file treat FILE as regular source file + --subjective, --strict enable more subjective tests + --ignore TYPE(,TYPE2...) ignore various comma separated message types + --show-types show the message "types" in the output + --root=PATH PATH to the kernel tree root + --no-summary suppress the per-file summary + --mailback only produce a report in case of warnings/errors + --summary-file include the filename in summary + --debug KEY=[0|1] turn on/off debugging of KEY, where KEY is one of + 'values', 'possible', 'type', and 'attr' (default + is all off) + --test-only=WORD report only warnings/errors containing WORD + literally + -h, --help, --version display this help and exit + +When FILE is - read standard input. +EOM + + exit($exitcode); +} + +my $conf = which_conf($configuration_file); +if (-f $conf) { + my @conf_args; + open(my $conffile, '<', "$conf") + or warn "$P: Can't find a readable $configuration_file file $!\n"; + + while (<$conffile>) { + my $line = $_; + + $line =~ s/\s*\n?$//g; + $line =~ s/^\s*//g; + $line =~ s/\s+/ /g; + + next if ($line =~ m/^\s*#/); + next if ($line =~ m/^\s*$/); + + my @words = split(" ", $line); + foreach my $word (@words) { + last if ($word =~ m/^#/); + push (@conf_args, $word); + } + } + close($conffile); + unshift(@ARGV, @conf_args) if @conf_args; +} + GetOptions( 'q|quiet+' => \$quiet, 'tree!' => \$tree, @@ -35,9 +102,11 @@ GetOptions( 'patch!' => \$chk_patch, 'emacs!' => \$emacs, 'terse!' => \$terse, - 'file!' => \$file, + 'f|file!' => \$file, 'subjective!' => \$check, 'strict!' => \$check, + 'ignore=s' => \@ignore, + 'show-types!' => \$show_types, 'root=s' => \$root, 'summary!' => \$summary, 'mailback!' => \$mailback, @@ -45,25 +114,32 @@ GetOptions( 'debug=s' => \%debug, 'test-only=s' => \$tst_only, -) or exit; + 'h|help' => \$help, + 'version' => \$help +) or help(1); + +help(0) if ($help); my $exit = 0; if ($#ARGV < 0) { - print "usage: $P [options] patchfile\n"; - print "version: $V\n"; - print "options: -q => quiet\n"; - print " --no-tree => run without a kernel tree\n"; - print " --terse => one line per report\n"; - print " --emacs => emacs compile window format\n"; - print " --file => check a source file\n"; - print " --strict => enable more subjective tests\n"; - print " --root => path to the kernel tree root\n"; - print " --no-summary => suppress the per-file summary\n"; - print " --summary-file => include the filename in summary\n"; + print "$P: no input files\n"; exit(1); } +@ignore = split(/,/, join(',',@ignore)); +foreach my $word (@ignore) { + $word =~ s/\s*\n?$//g; + $word =~ s/^\s*//g; + $word =~ s/\s+/ /g; + $word =~ tr/[a-z]/[A-Z]/; + + next if ($word =~ m/^\s*#/); + next if ($word =~ m/^\s*$/); + + $ignore_type{$word}++; +} + my $dbg_values = 0; my $dbg_possible = 0; my $dbg_type = 0; @@ -74,6 +150,8 @@ for my $key (keys %debug) { die "$@" if ($@); } +my $rpt_cleaners = 0; + if ($terse) { $emacs = 1; $quiet++; @@ -101,7 +179,10 @@ if ($tree) { my $emitted_corrupt = 0; -our $Ident = qr{[A-Za-z_][A-Za-z\d_]*}; +our $Ident = qr{ + [A-Za-z_][A-Za-z\d_]* + (?:\s*\#\#\s*[A-Za-z_][A-Za-z\d_]*)* + }x; our $Storage = qr{extern|static|asmlinkage}; our $Sparse = qr{ __user| @@ -111,13 +192,31 @@ our $Sparse = qr{ __must_check| __init_refok| __kprobes| - __ref + __ref| + __rcu }x; + +# Notes to $Attribute: +# We need \b after 'init' otherwise 'initconst' will cause a false positive in a check our $Attribute = qr{ const| + __percpu| + __nocast| + __safe| + __bitwise__| + __packed__| + __packed2__| + __naked| + __maybe_unused| + __always_unused| + __noreturn| + __used| + __cold| + __noclone| + __deprecated| __read_mostly| __kprobes| - __(?:mem|cpu|dev|)(?:initdata|init)| + __(?:mem|cpu|dev|)(?:initdata|initconst|init\b)| ____cacheline_aligned| ____cacheline_aligned_in_smp| ____cacheline_internodealigned_in_smp| @@ -128,7 +227,7 @@ our $Inline = qr{inline|__always_inline|noinline}; our $Member = qr{->$Ident|\.$Ident|\[[^]]*\]}; our $Lval = qr{$Ident(?:$Member)*}; -our $Constant = qr{(?:[0-9]+|0x[0-9a-fA-F]+)[UL]*}; +our $Constant = qr{(?i:(?:[0-9]+|0x[0-9a-f]+)[ul]*)}; our $Assignment = qr{(?:\*\=|/=|%=|\+=|-=|<<=|>>=|&=|\^=|\|=|=)}; our $Compare = qr{<=|>=|==|!=|<|>}; our $Operators = qr{ @@ -141,9 +240,8 @@ our $NonptrType; our $Type; our $Declare; -our $UTF8 = qr { - [\x09\x0A\x0D\x20-\x7E] # ASCII - | [\xC2-\xDF][\x80-\xBF] # non-overlong 2-byte +our $NON_ASCII_UTF8 = qr{ + [\xC2-\xDF][\x80-\xBF] # non-overlong 2-byte | \xE0[\xA0-\xBF][\x80-\xBF] # excluding overlongs | [\xE1-\xEC\xEE\xEF][\x80-\xBF]{2} # straight 3-byte | \xED[\x80-\x9F][\x80-\xBF] # excluding surrogates @@ -152,11 +250,34 @@ our $UTF8 = qr { | \xF4[\x80-\x8F][\x80-\xBF]{2} # plane 16 }x; +our $UTF8 = qr{ + [\x09\x0A\x0D\x20-\x7E] # ASCII + | $NON_ASCII_UTF8 +}x; + our $typeTypedefs = qr{(?x: - (?:__)?(?:u|s|be|le)(?:\d|\d\d)| + (?:__)?(?:u|s|be|le)(?:8|16|32|64)| atomic_t )}; +our $logFunctions = qr{(?x: + printk(?:_ratelimited|_once|)| + [a-z0-9]+_(?:printk|emerg|alert|crit|err|warning|warn|notice|info|debug|dbg|vdbg|devel|cont|WARN)(?:_ratelimited|_once|)| + WARN(?:_RATELIMIT|_ONCE|)| + panic| + MODULE_[A-Z_]+ +)}; + +our $signature_tags = qr{(?xi: + Signed-off-by:| + Acked-by:| + Tested-by:| + Reviewed-by:| + Reported-by:| + To:| + Cc: +)}; + our @typeList = ( qr{void}, qr{(?:unsigned\s+)?char}, @@ -181,6 +302,12 @@ our @modifierList = ( qr{fastcall}, ); +our $allowed_asm_includes = qr{(?x: + irq| + memory +)}; +# memory.h: ARM has a custom one + sub build_types { my $mods = "(?x: \n" . join("|\n ", @modifierList) . "\n)"; my $all = "(?x: \n" . join("|\n ", @typeList) . "\n)"; @@ -188,7 +315,7 @@ sub build_types { $NonptrType = qr{ (?:$Modifier\s+|const\s+)* (?: - (?:typeof|__typeof__)\s*\(\s*\**\s*$Ident\s*\)| + (?:typeof|__typeof__)\s*\([^\)]*\)| (?:$typeTypedefs\b)| (?:${all}\b) ) @@ -196,13 +323,33 @@ sub build_types { }x; $Type = qr{ $NonptrType - (?:[\s\*]+\s*const|[\s\*]+|(?:\s*\[\s*\])+)? + (?:(?:\s|\*|\[\])+\s*const|(?:\s|\*|\[\])+|(?:\s*\[\s*\])+)? (?:\s+$Inline|\s+$Modifier)* }x; $Declare = qr{(?:$Storage\s+)?$Type}; } build_types(); + +our $Typecast = qr{\s*(\(\s*$NonptrType\s*\)){0,1}\s*}; + +# Using $balanced_parens, $LvalOrFunc, or $FuncArg +# requires at least perl version v5.10.0 +# Any use must be runtime checked with $^V + +our $balanced_parens = qr/(\((?:[^\(\)]++|(?-1))*\))/; +our $LvalOrFunc = qr{($Lval)\s*($balanced_parens{0,1})\s*}; +our $FuncArg = qr{$Typecast{0,1}($LvalOrFunc|$Constant)}; + +sub deparenthesize { + my ($string) = @_; + return "" if (!defined($string)); + $string =~ s@^\s*\(\s*@@g; + $string =~ s@\s*\)\s*$@@g; + $string =~ s@\s+@ @g; + return $string; +} + $chk_signoff = 0 if ($file); my @dep_includes = (); @@ -274,6 +421,88 @@ sub top_of_kernel_tree { } } return 1; + } + +sub parse_email { + my ($formatted_email) = @_; + + my $name = ""; + my $address = ""; + my $comment = ""; + + if ($formatted_email =~ /^(.*)<(\S+\@\S+)>(.*)$/) { + $name = $1; + $address = $2; + $comment = $3 if defined $3; + } elsif ($formatted_email =~ /^\s*<(\S+\@\S+)>(.*)$/) { + $address = $1; + $comment = $2 if defined $2; + } elsif ($formatted_email =~ /(\S+\@\S+)(.*)$/) { + $address = $1; + $comment = $2 if defined $2; + $formatted_email =~ s/$address.*$//; + $name = $formatted_email; + $name =~ s/^\s+|\s+$//g; + $name =~ s/^\"|\"$//g; + # If there's a name left after stripping spaces and + # leading quotes, and the address doesn't have both + # leading and trailing angle brackets, the address + # is invalid. ie: + # "joe smith joe@smith.com" bad + # "joe smith ]+>$/) { + $name = ""; + $address = ""; + $comment = ""; + } + } + + $name =~ s/^\s+|\s+$//g; + $name =~ s/^\"|\"$//g; + $address =~ s/^\s+|\s+$//g; + $address =~ s/^\<|\>$//g; + + if ($name =~ /[^\w \-]/i) { ##has "must quote" chars + $name =~ s/(?"; + } + + return $formatted_email; +} + +sub which_conf { + my ($conf) = @_; + + foreach my $path (split(/:/, ".:$ENV{HOME}:.scripts")) { + if (-e "$path/$conf") { + return "$path/$conf"; + } + } + + return ""; } sub expand_tabs { @@ -356,6 +585,13 @@ sub sanitise_line { $off++; next; } + if ($sanitise_quote eq '' && substr($line, $off, 2) eq '//') { + $sanitise_quote = '//'; + + substr($res, $off, 2, $sanitise_quote); + $off++; + next; + } # A \ in a string means ignore the next character. if (($sanitise_quote eq "'" || $sanitise_quote eq '"') && @@ -379,6 +615,8 @@ sub sanitise_line { #print "c<$c> SQ<$sanitise_quote>\n"; if ($off != 0 && $sanitise_quote eq '*/' && $c ne "\t") { substr($res, $off, 1, $;); + } elsif ($off != 0 && $sanitise_quote eq '//' && $c ne "\t") { + substr($res, $off, 1, $;); } elsif ($off != 0 && $sanitise_quote && $c ne "\t") { substr($res, $off, 1, 'X'); } else { @@ -386,6 +624,10 @@ sub sanitise_line { } } + if ($sanitise_quote eq '//') { + $sanitise_quote = ''; + } + # The pathname on a #include may be surrounded by '<' and '>'. if ($res =~ /^.\s*\#\s*include\s+\<(.*)\>/) { my $clean = 'X' x length($1); @@ -440,6 +682,10 @@ sub ctx_statement_block { if ($off >= $len) { last; } + if ($level == 0 && substr($blk, $off) =~ /^.\s*#\s*define/) { + $level++; + $type = '#'; + } } $p = $c; $c = substr($blk, $off, 1); @@ -496,9 +742,19 @@ sub ctx_statement_block { $type = ($level != 0)? '{' : ''; if ($level == 0) { + if (substr($blk, $off + 1, 1) eq ';') { + $off++; + } last; } } + # Preprocessor commands end at the newline unless escaped. + if ($type eq '#' && $c eq "\n" && $p ne "\\") { + $level--; + $type = ''; + $off++; + last; + } $off++; } # We are truly at the end, so shuffle to the next line. @@ -610,15 +866,15 @@ sub ctx_block_get { $blk .= $rawlines[$line]; # Handle nested #if/#else. - if ($rawlines[$line] =~ /^.\s*#\s*(?:ifndef|ifdef|if)\s/) { + if ($lines[$line] =~ /^.\s*#\s*(?:ifndef|ifdef|if)\s/) { push(@stack, $level); - } elsif ($rawlines[$line] =~ /^.\s*#\s*(?:else|elif)\b/) { + } elsif ($lines[$line] =~ /^.\s*#\s*(?:else|elif)\b/) { $level = $stack[$#stack - 1]; - } elsif ($rawlines[$line] =~ /^.\s*#\s*endif\b/) { + } elsif ($lines[$line] =~ /^.\s*#\s*endif\b/) { $level = pop(@stack); } - foreach my $c (split(//, $rawlines[$line])) { + foreach my $c (split(//, $lines[$line])) { ##print "C<$c>L<$level><$open$close>O<$off>\n"; if ($off > 0) { $off--; @@ -778,7 +1034,12 @@ sub annotate_values { $av_preprocessor = 0; } - } elsif ($cur =~ /^($Type)\s*(?:$Ident|,|\)|\()/) { + } elsif ($cur =~ /^(\(\s*$Type\s*)\)/ && $av_pending eq '_') { + print "CAST($1)\n" if ($dbg_values > 1); + push(@av_paren_type, $type); + $type = 'c'; + + } elsif ($cur =~ /^($Type)\s*(?:$Ident|,|\)|\(|\s*$)/) { print "DECLARE($1)\n" if ($dbg_values > 1); $type = 'T'; @@ -955,23 +1216,27 @@ sub annotate_values { sub possible { my ($possible, $line) = @_; - - print "CHECK<$possible> ($line)\n" if ($dbg_possible > 2); - if ($possible !~ /(?: + my $notPermitted = qr{(?: ^(?: $Modifier| $Storage| $Type| - DEFINE_\S+| + DEFINE_\S+ + )$| + ^(?: goto| return| case| else| asm|__asm__| - do - )$| + do| + \#| + \#\#| + )(?:\s|$)| ^(?:typedef|struct|enum)\b - )/x) { + )}x; + warn "CHECK<$possible> ($line)\n" if ($dbg_possible > 2); + if ($possible !~ $notPermitted) { # Check for modifiers. $possible =~ s/\s*$Storage\s*//g; $possible =~ s/\s*$Sparse\s*//g; @@ -980,8 +1245,10 @@ sub possible { } elsif ($possible =~ /\s/) { $possible =~ s/\s*$Type\s*//g; for my $modifier (split(' ', $possible)) { - warn "MODIFIER: $modifier ($possible) ($line)\n" if ($dbg_possible); - push(@modifierList, $modifier); + if ($modifier !~ $notPermitted) { + warn "MODIFIER: $modifier ($possible) ($line)\n" if ($dbg_possible); + push(@modifierList, $modifier); + } } } else { @@ -996,12 +1263,21 @@ sub possible { my $prefix = ''; +sub show_type { + return !defined $ignore_type{$_[0]}; +} + sub report { - if (defined $tst_only && $_[0] !~ /\Q$tst_only\E/) { + if (!show_type($_[1]) || + (defined $tst_only && $_[2] !~ /\Q$tst_only\E/)) { return 0; } - my $line = $prefix . $_[0]; - + my $line; + if ($show_types) { + $line = "$prefix$_[0]:$_[1]: $_[2]\n"; + } else { + $line = "$prefix$_[0]: $_[2]\n"; + } $line = (split('\n', $line))[0] . "\n" if ($terse); push(our @report, $line); @@ -1011,20 +1287,21 @@ sub report { sub report_dump { our @report; } + sub ERROR { - if (report("ERROR: $_[0]\n")) { + if (report("ERROR", $_[0], $_[1])) { our $clean = 0; our $cnt_error++; } } sub WARN { - if (report("WARNING: $_[0]\n")) { + if (report("WARNING", $_[0], $_[1])) { our $clean = 0; our $cnt_warn++; } } sub CHK { - if ($check && report("CHECK: $_[0]\n")) { + if ($check && report("CHECK", $_[0], $_[1])) { our $clean = 0; our $cnt_chk++; } @@ -1053,10 +1330,41 @@ sub check_absolute_file { ##print "prefix<$prefix>\n"; if ($prefix ne ".../") { - WARN("use relative pathname instead of absolute in changelog text\n" . $herecurr); + WARN("USE_RELATIVE_PATH", + "use relative pathname instead of absolute in changelog text\n" . $herecurr); } } +sub pos_last_openparen { + my ($line) = @_; + + my $pos = 0; + + my $opens = $line =~ tr/\(/\(/; + my $closes = $line =~ tr/\)/\)/; + + my $last_openparen = 0; + + if (($opens == 0) || ($closes >= $opens)) { + return -1; + } + + my $len = length($line); + + for ($pos = 0; $pos < $len; $pos++) { + my $string = substr($line, $pos); + if ($string =~ /^($FuncArg|$balanced_parens)/) { + $pos += length($1) - 1; + } elsif (substr($line, $pos, 1) eq '(') { + $last_openparen = $pos; + } elsif (index($string, '(') == -1) { + last; + } + } + + return $last_openparen + 1; +} + sub process { my $filename = shift; @@ -1075,6 +1383,9 @@ sub process { my $signoff = 0; my $is_patch = 0; + my $in_header_lines = 1; + my $in_commit_log = 0; #Scanning lines before patch + our @report = (); our $cnt_lines = 0; our $cnt_error = 0; @@ -1096,6 +1407,8 @@ sub process { # suppression flags my %suppress_ifbraces; my %suppress_whiletrailers; + my %suppress_export; + my $suppress_statement = 0; # Pre-scan the patch sanitizing the lines. # Pre-scan the patch looking for any __setup documentation. @@ -1188,7 +1501,6 @@ sub process { $linenr++; my $rawline = $rawlines[$linenr - 1]; - my $hunk_line = ($realcnt != 0); #extract the line range in the file after the patch is applied if ($line=~/^\@\@ -\d+(?:,\d+)? \+(\d+)(,(\d+))? \@\@/) { @@ -1205,6 +1517,8 @@ sub process { %suppress_ifbraces = (); %suppress_whiletrailers = (); + %suppress_export = (); + $suppress_statement = 0; next; # track the line number as we move through the hunk, note that @@ -1228,6 +1542,8 @@ sub process { $realcnt--; } + my $hunk_line = ($realcnt != 0); + #make up the handle for any error we report on this line $prefix = "$filename:$realline: " if ($emacs && $file); $prefix = "$filename:$linenr: " if ($emacs && !$file); @@ -1236,18 +1552,25 @@ sub process { $here = "#$realline: " if ($file); # extract the filename as it passes - if ($line=~/^\+\+\+\s+(\S+)/) { + if ($line =~ /^diff --git.*?(\S+)$/) { $realfile = $1; $realfile =~ s@^([^/]*)/@@; + $in_commit_log = 0; + } elsif ($line =~ /^\+\+\+\s+(\S+)/) { + $realfile = $1; + $realfile =~ s@^([^/]*)/@@; + $in_commit_log = 0; $p1_prefix = $1; if (!$file && $tree && $p1_prefix ne '' && -e "$root/$p1_prefix") { - WARN("patch prefix '$p1_prefix' exists, appears to be a -p0 patch\n"); + WARN("PATCH_PREFIX", + "patch prefix '$p1_prefix' exists, appears to be a -p0 patch\n"); } if ($realfile =~ m@^include/asm/@) { - ERROR("do not modify files in include/asm, change architecture specific files in include/asm-\n" . "$here$rawline\n"); + ERROR("MODIFIED_INCLUDE_ASM", + "do not modify files in include/asm, change architecture specific files in include/asm-\n" . "$here$rawline\n"); } next; } @@ -1260,23 +1583,67 @@ sub process { $cnt_lines++ if ($realcnt != 0); -#check the patch for a signoff: - if ($line =~ /^\s*signed-off-by:/i) { - # This is a signoff, if ugly, so do not double report. - $signoff++; - if (!($line =~ /^\s*Signed-off-by:/)) { - WARN("Signed-off-by: is the preferred form\n" . - $herecurr); +# Check for incorrect file permissions + if ($line =~ /^new (file )?mode.*[7531]\d{0,2}$/) { + my $permhere = $here . "FILE: $realfile\n"; + if ($realfile =~ /(Makefile|Kconfig|\.c|\.h|\.S|\.tmpl)$/) { + ERROR("EXECUTE_PERMISSIONS", + "do not set execute permissions for source files\n" . $permhere); } - if ($line =~ /^\s*signed-off-by:\S/i) { - WARN("space required after Signed-off-by:\n" . - $herecurr); + } + +# Check the patch for a signoff: + if ($line =~ /^\s*signed-off-by:/i) { + $signoff++; + $in_commit_log = 0; + } + +# Check signature styles + if (!$in_header_lines && + $line =~ /^(\s*)($signature_tags)(\s*)(.*)/) { + my $space_before = $1; + my $sign_off = $2; + my $space_after = $3; + my $email = $4; + my $ucfirst_sign_off = ucfirst(lc($sign_off)); + + if (defined $space_before && $space_before ne "") { + WARN("BAD_SIGN_OFF", + "Do not use whitespace before $ucfirst_sign_off\n" . $herecurr); + } + if ($sign_off =~ /-by:$/i && $sign_off ne $ucfirst_sign_off) { + WARN("BAD_SIGN_OFF", + "'$ucfirst_sign_off' is the preferred signature form\n" . $herecurr); + } + if (!defined $space_after || $space_after ne " ") { + WARN("BAD_SIGN_OFF", + "Use a single space after $ucfirst_sign_off\n" . $herecurr); + } + + my ($email_name, $email_address, $comment) = parse_email($email); + my $suggested_email = format_email(($email_name, $email_address)); + if ($suggested_email eq "") { + ERROR("BAD_SIGN_OFF", + "Unrecognized email address: '$email'\n" . $herecurr); + } else { + my $dequoted = $suggested_email; + $dequoted =~ s/^"//; + $dequoted =~ s/" $comment" ne $email && + "$suggested_email$comment" ne $email) { + WARN("BAD_SIGN_OFF", + "email address '$email' might be better as '$suggested_email$comment'\n" . $herecurr); + } } } # Check for wrappage within a valid hunk of the file if ($realcnt != 0 && $line !~ m{^(?:\+|-| |\\ No newline|$)}) { - ERROR("patch seems to be corrupt (line wrapped?)\n" . + ERROR("CORRUPTED_PATCH", + "patch seems to be corrupt (line wrapped?)\n" . $herecurr) if (!$emitted_corrupt++); } @@ -1303,7 +1670,23 @@ sub process { my $ptr = substr($blank, 0, length($utf8_prefix)) . "^"; my $hereptr = "$hereline$ptr\n"; - ERROR("Invalid UTF-8, patch and commit message should be encoded in UTF-8\n" . $hereptr); + CHK("INVALID_UTF8", + "Invalid UTF-8, patch and commit message should be encoded in UTF-8\n" . $hereptr); + } + +# Check if it's the start of a commit log +# (not a header line and we haven't seen the patch filename) + if ($in_header_lines && $realfile =~ /^$/ && + $rawline !~ /^(commit\b|from\b|[\w-]+:).+$/i) { + $in_header_lines = 0; + $in_commit_log = 1; + } + +# Still not yet in a patch, check for any UTF-8 + if ($in_commit_log && $realfile =~ /^$/ && + $rawline =~ /$NON_ASCII_UTF8/) { + CHK("UTF8_BEFORE_PATCH", + "8-bit UTF-8 used in possible commit log\n" . $herecurr); } # ignore non-hunk lines and lines being removed @@ -1312,11 +1695,67 @@ sub process { #trailing whitespace if ($line =~ /^\+.*\015/) { my $herevet = "$here\n" . cat_vet($rawline) . "\n"; - ERROR("DOS line endings\n" . $herevet); + ERROR("DOS_LINE_ENDINGS", + "DOS line endings\n" . $herevet); } elsif ($rawline =~ /^\+.*\S\s+$/ || $rawline =~ /^\+\s+$/) { my $herevet = "$here\n" . cat_vet($rawline) . "\n"; - ERROR("trailing whitespace\n" . $herevet); + ERROR("TRAILING_WHITESPACE", + "trailing whitespace\n" . $herevet); + $rpt_cleaners = 1; + } + +# check for Kconfig help text having a real description +# Only applies when adding the entry originally, after that we do not have +# sufficient context to determine whether it is indeed long enough. + if ($realfile =~ /Kconfig/ && + $line =~ /.\s*config\s+/) { + my $length = 0; + my $cnt = $realcnt; + my $ln = $linenr + 1; + my $f; + my $is_start = 0; + my $is_end = 0; + for (; $cnt > 0 && defined $lines[$ln - 1]; $ln++) { + $f = $lines[$ln - 1]; + $cnt-- if ($lines[$ln - 1] !~ /^-/); + $is_end = $lines[$ln - 1] =~ /^\+/; + + next if ($f =~ /^-/); + + if ($lines[$ln - 1] =~ /.\s*(?:bool|tristate)\s*\"/) { + $is_start = 1; + } elsif ($lines[$ln - 1] =~ /.\s*(?:---)?help(?:---)?$/) { + $length = -1; + } + + $f =~ s/^.//; + $f =~ s/#.*//; + $f =~ s/^\s+//; + next if ($f =~ /^$/); + if ($f =~ /^\s*config\s/) { + $is_end = 1; + last; + } + $length++; + } + WARN("CONFIG_DESCRIPTION", + "please write a paragraph that describes the config symbol fully\n" . $herecurr) if ($is_start && $is_end && $length < 4); + #print "is_start<$is_start> is_end<$is_end> length<$length>\n"; + } + + if (($realfile =~ /Makefile.*/ || $realfile =~ /Kbuild.*/) && + ($line =~ /\+(EXTRA_[A-Z]+FLAGS).*/)) { + my $flag = $1; + my $replacement = { + 'EXTRA_AFLAGS' => 'asflags-y', + 'EXTRA_CFLAGS' => 'ccflags-y', + 'EXTRA_CPPFLAGS' => 'cppflags-y', + 'EXTRA_LDFLAGS' => 'ldflags-y', + }; + + WARN("DEPRECATED_VARIABLE", + "Use of $flag is deprecated, please use \`$replacement->{$flag} instead.\n" . $herecurr) if ($replacement->{$flag}); } # check we are in a valid source file if not then ignore this hunk @@ -1325,15 +1764,53 @@ sub process { #80 column limit if ($line =~ /^\+/ && $prevrawline !~ /\/\*\*/ && $rawline !~ /^.\s*\*\s*\@$Ident\s/ && - $line !~ /^\+\s*printk\s*\(\s*(?:KERN_\S+\s*)?"[X\t]*"\s*(?:,|\)\s*;)\s*$/ && + !($line =~ /^\+\s*$logFunctions\s*\(\s*(?:(KERN_\S+\s*|[^"]*))?"[X\t]*"\s*(?:|,|\)\s*;)\s*$/ || + $line =~ /^\+\s*"[^"]*"\s*(?:\s*|,|\)\s*;)\s*$/) && $length > 80) { - WARN("line over 80 characters\n" . $herecurr); + WARN("LONG_LINE", + "line over 80 characters\n" . $herecurr); + } + +# Check for user-visible strings broken across lines, which breaks the ability +# to grep for the string. Limited to strings used as parameters (those +# following an open parenthesis), which almost completely eliminates false +# positives, as well as warning only once per parameter rather than once per +# line of the string. Make an exception when the previous string ends in a +# newline (multiple lines in one string constant) or \n\t (common in inline +# assembly to indent the instruction on the following line). + if ($line =~ /^\+\s*"/ && + $prevline =~ /"\s*$/ && + $prevline =~ /\(/ && + $prevrawline !~ /\\n(?:\\t)*"\s*$/) { + WARN("SPLIT_STRING", + "quoted string split across lines\n" . $hereprev); + } + +# check for spaces before a quoted newline + if ($rawline =~ /^.*\".*\s\\n/) { + WARN("QUOTED_WHITESPACE_BEFORE_NEWLINE", + "unnecessary whitespace before a quoted newline\n" . $herecurr); } # check for adding lines without a newline. if ($line =~ /^\+/ && defined $lines[$linenr] && $lines[$linenr] =~ /^\\ No newline at end of file/) { - WARN("adding a line without newline at end of file\n" . $herecurr); + WARN("MISSING_EOF_NEWLINE", + "adding a line without newline at end of file\n" . $herecurr); + } + +# Blackfin: use hi/lo macros + if ($realfile =~ m@arch/blackfin/.*\.S$@) { + if ($line =~ /\.[lL][[:space:]]*=.*&[[:space:]]*0x[fF][fF][fF][fF]/) { + my $herevet = "$here\n" . cat_vet($line) . "\n"; + ERROR("LO_MACRO", + "use the LO() macro, not (... & 0xFFFF)\n" . $herevet); + } + if ($line =~ /\.[hH][[:space:]]*=.*>>[[:space:]]*16/) { + my $herevet = "$here\n" . cat_vet($line) . "\n"; + ERROR("HI_MACRO", + "use the HI() macro, not (... >> 16)\n" . $herevet); + } } # check we are in a valid source file C or perl if not then ignore this hunk @@ -1344,7 +1821,69 @@ sub process { if ($rawline =~ /^\+\s* \t\s*\S/ || $rawline =~ /^\+\s* \s*/) { my $herevet = "$here\n" . cat_vet($rawline) . "\n"; - ERROR("code indent should use tabs where possible\n" . $herevet); + ERROR("CODE_INDENT", + "code indent should use tabs where possible\n" . $herevet); + $rpt_cleaners = 1; + } + +# check for space before tabs. + if ($rawline =~ /^\+/ && $rawline =~ / \t/) { + my $herevet = "$here\n" . cat_vet($rawline) . "\n"; + WARN("SPACE_BEFORE_TAB", + "please, no space before tabs\n" . $herevet); + } + +# check for && or || at the start of a line + if ($rawline =~ /^\+\s*(&&|\|\|)/) { + CHK("LOGICAL_CONTINUATIONS", + "Logical continuations should be on the previous line\n" . $hereprev); + } + +# check multi-line statement indentation matches previous line + if ($^V && $^V ge 5.10.0 && + $prevline =~ /^\+(\t*)(if \(|$Ident\().*(\&\&|\|\||,)\s*$/) { + $prevline =~ /^\+(\t*)(.*)$/; + my $oldindent = $1; + my $rest = $2; + + my $pos = pos_last_openparen($rest); + if ($pos >= 0) { + $line =~ /^\+([ \t]*)/; + my $newindent = $1; + + my $goodtabindent = $oldindent . + "\t" x ($pos / 8) . + " " x ($pos % 8); + my $goodspaceindent = $oldindent . " " x $pos; + + if ($newindent ne $goodtabindent && + $newindent ne $goodspaceindent) { + CHK("PARENTHESIS_ALIGNMENT", + "Alignment should match open parenthesis\n" . $hereprev); + } + } + } + + if ($line =~ /^\+.*\*[ \t]*\)[ \t]+/) { + CHK("SPACING", + "No space is necessary after a cast\n" . $hereprev); + } + + if ($rawline =~ /^\+[ \t]*\/\*[ \t]*$/ && + $prevrawline =~ /^\+[ \t]*$/) { + CHK("BLOCK_COMMENT_STYLE", + "Don't begin block comments with only a /* line, use /* comment...\n" . $hereprev); + } + +# check for spaces at the beginning of a line. +# Exceptions: +# 1) within comments +# 2) indented preprocessor commands +# 3) hanging labels + if ($rawline =~ /^\+ / && $line !~ /\+ *(?:$;|#|$Ident:)/) { + my $herevet = "$here\n" . cat_vet($rawline) . "\n"; + WARN("LEADING_SPACE", + "please, no spaces at the start of a line\n" . $herevet); } # check we are in a valid C source file if not then ignore this hunk @@ -1352,17 +1891,51 @@ sub process { # check for RCS/CVS revision markers if ($rawline =~ /^\+.*\$(Revision|Log|Id)(?:\$|)/) { - WARN("CVS style keyword markers, these will _not_ be updated\n". $herecurr); + WARN("CVS_KEYWORD", + "CVS style keyword markers, these will _not_ be updated\n". $herecurr); + } + +# Blackfin: don't use __builtin_bfin_[cs]sync + if ($line =~ /__builtin_bfin_csync/) { + my $herevet = "$here\n" . cat_vet($line) . "\n"; + ERROR("CSYNC", + "use the CSYNC() macro in asm/blackfin.h\n" . $herevet); + } + if ($line =~ /__builtin_bfin_ssync/) { + my $herevet = "$here\n" . cat_vet($line) . "\n"; + ERROR("SSYNC", + "use the SSYNC() macro in asm/blackfin.h\n" . $herevet); } # Check for potential 'bare' types - my ($stat, $cond, $line_nr_next, $remain_next, $off_next); - if ($realcnt && $line =~ /.\s*\S/) { + my ($stat, $cond, $line_nr_next, $remain_next, $off_next, + $realline_next); +#print "LINE<$line>\n"; + if ($linenr >= $suppress_statement && + $realcnt && $line =~ /.\s*\S/) { ($stat, $cond, $line_nr_next, $remain_next, $off_next) = ctx_statement_block($linenr, $realcnt, 0); $stat =~ s/\n./\n /g; $cond =~ s/\n./\n /g; +#print "linenr<$linenr> <$stat>\n"; + # If this statement has no statement boundaries within + # it there is no point in retrying a statement scan + # until we hit end of it. + my $frag = $stat; $frag =~ s/;+\s*$//; + if ($frag !~ /(?:{|;)/) { +#print "skip<$line_nr_next>\n"; + $suppress_statement = $line_nr_next; + } + + # Find the real next line. + $realline_next = $line_nr_next; + if (defined $realline_next && + (!defined $lines[$realline_next - 1] || + substr($lines[$realline_next - 1], $off_next) =~ /^\s*$/)) { + $realline_next++; + } + my $s = $stat; $s =~ s/{.*$//s; @@ -1372,6 +1945,8 @@ sub process { # Ignore functions being called } elsif ($s =~ /^.\s*$Ident\s*\(/s) { + } elsif ($s =~ /^.\s*else\b/s) { + # declarations always start with types } elsif ($prev_values eq 'E' && $s =~ /^.\s*(?:$Storage\s+)?(?:$Inline\s+)?(?:const\s+)?((?:\s*$Ident)+?)\b(?:\s+$Sparse)?\s*\**\s*(?:$Ident|\(\*[^\)]*\))(?:\s*$Modifier)?\s*(?:;|=|,|\()/s) { my $type = $1; @@ -1430,7 +2005,8 @@ sub process { } } if ($err ne '') { - ERROR("switch and case should be at the same indent\n$hereline$err"); + ERROR("SWITCH_CASE_INDENT_LEVEL", + "switch and case should be at the same indent\n$hereline$err"); } } @@ -1440,6 +2016,12 @@ sub process { my $pre_ctx = "$1$2"; my ($level, @ctx) = ctx_statement_level($linenr, $realcnt, 0); + + if ($line =~ /^\+\t{6,}/) { + WARN("DEEP_INDENTATION", + "Too many leading tabs - consider code refactoring\n" . $herecurr); + } + my $ctx_cnt = $realcnt - $#ctx - 1; my $ctx = join("\n", @ctx); @@ -1458,8 +2040,9 @@ sub process { #print "pre<$pre_ctx>\nline<$line>\nctx<$ctx>\nnext<$lines[$ctx_ln - 1]>\n"; if ($ctx !~ /{\s*/ && defined($lines[$ctx_ln -1]) && $lines[$ctx_ln - 1] =~ /^\+\s*{/) { - ERROR("that open brace { should be on the previous line\n" . - "$here\n$ctx\n$lines[$ctx_ln - 1]\n"); + ERROR("OPEN_BRACE", + "that open brace { should be on the previous line\n" . + "$here\n$ctx\n$rawlines[$ctx_ln - 1]\n"); } if ($level == 0 && $pre_ctx !~ /}\s*while\s*\($/ && $ctx =~ /\)\s*\;\s*$/ && @@ -1467,14 +2050,18 @@ sub process { { my ($nlength, $nindent) = line_stats($lines[$ctx_ln - 1]); if ($nindent > $indent) { - WARN("trailing semicolon indicates no statements, indent implies otherwise\n" . - "$here\n$ctx\n$lines[$ctx_ln - 1]\n"); + WARN("TRAILING_SEMICOLON", + "trailing semicolon indicates no statements, indent implies otherwise\n" . + "$here\n$ctx\n$rawlines[$ctx_ln - 1]\n"); } } } # Check relative indent for conditionals and blocks. if ($line =~ /\b(?:(?:if|while|for)\s*\(|do\b)/ && $line !~ /^.\s*#/ && $line !~ /\}\s*while\s*/) { + ($stat, $cond, $line_nr_next, $remain_next, $off_next) = + ctx_statement_block($linenr, $realcnt, 0) + if (!defined $stat); my ($s, $c) = ($stat, $cond); substr($s, 0, length($c), ''); @@ -1532,8 +2119,9 @@ sub process { $s =~ /^\s*#\s*?/ || $s =~ /^\s*$Ident\s*:/) { $continuation = ($s =~ /^.*?\\\n/) ? 1 : 0; - $s =~ s/^.*?\n//; - $cond_lines++; + if ($s =~ s/^.*?\n//) { + $cond_lines++; + } } } @@ -1554,7 +2142,8 @@ sub process { if ($check && (($sindent % 8) != 0 || ($sindent <= $indent && $s ne ''))) { - WARN("suspect code indent for conditional statements ($indent, $sindent)\n" . $herecurr . "$stat_real\n"); + WARN("SUSPECT_CODE_INDENT", + "suspect code indent for conditional statements ($indent, $sindent)\n" . $herecurr . "$stat_real\n"); } } @@ -1577,26 +2166,31 @@ sub process { # TEST: allow direct testing of the type matcher. if ($dbg_type) { if ($line =~ /^.\s*$Declare\s*$/) { - ERROR("TEST: is type\n" . $herecurr); + ERROR("TEST_TYPE", + "TEST: is type\n" . $herecurr); } elsif ($dbg_type > 1 && $line =~ /^.+($Declare)/) { - ERROR("TEST: is not type ($1 is)\n". $herecurr); + ERROR("TEST_NOT_TYPE", + "TEST: is not type ($1 is)\n". $herecurr); } next; } # TEST: allow direct testing of the attribute matcher. if ($dbg_attr) { if ($line =~ /^.\s*$Modifier\s*$/) { - ERROR("TEST: is attr\n" . $herecurr); + ERROR("TEST_ATTR", + "TEST: is attr\n" . $herecurr); } elsif ($dbg_attr > 1 && $line =~ /^.+($Modifier)/) { - ERROR("TEST: is not attr ($1 is)\n". $herecurr); + ERROR("TEST_NOT_ATTR", + "TEST: is not attr ($1 is)\n". $herecurr); } next; } # check for initialisation to aggregates open brace on the next line - if ($prevline =~ /$Declare\s*$Ident\s*=\s*$/ && - $line =~ /^.\s*{/) { - ERROR("that open brace { should be on the previous line\n" . $hereprev); + if ($line =~ /^.\s*{/ && + $prevline =~ /(?:^|[^=])=\s*$/) { + ERROR("OPEN_BRACE", + "that open brace { should be on the previous line\n" . $hereprev); } # @@ -1607,46 +2201,99 @@ sub process { if ($rawline =~ m{^.\s*\#\s*include\s+[<"](.*)[">]}) { my $path = $1; if ($path =~ m{//}) { - ERROR("malformed #include filename\n" . + ERROR("MALFORMED_INCLUDE", + "malformed #include filename\n" . $herecurr); } } # no C99 // comments if ($line =~ m{//}) { - ERROR("do not use C99 // comments\n" . $herecurr); + ERROR("C99_COMMENTS", + "do not use C99 // comments\n" . $herecurr); } # Remove C99 comments. $line =~ s@//.*@@; $opline =~ s@//.*@@; -#EXPORT_SYMBOL should immediately follow its function closing }. - if (($line =~ /EXPORT_SYMBOL.*\((.*)\)/) || - ($line =~ /EXPORT_UNUSED_SYMBOL.*\((.*)\)/)) { +# EXPORT_SYMBOL should immediately follow the thing it is exporting, consider +# the whole statement. +#print "APW <$lines[$realline_next - 1]>\n"; + if (defined $realline_next && + exists $lines[$realline_next - 1] && + !defined $suppress_export{$realline_next} && + ($lines[$realline_next - 1] =~ /EXPORT_SYMBOL.*\((.*)\)/ || + $lines[$realline_next - 1] =~ /EXPORT_UNUSED_SYMBOL.*\((.*)\)/)) { + # Handle definitions which produce identifiers with + # a prefix: + # XXX(foo); + # EXPORT_SYMBOL(something_foo); my $name = $1; - if ($prevline !~ /(?: - ^.}| + if ($stat =~ /^(?:.\s*}\s*\n)?.([A-Z_]+)\s*\(\s*($Ident)/ && + $name =~ /^${Ident}_$2/) { +#print "FOO C name<$name>\n"; + $suppress_export{$realline_next} = 1; + + } elsif ($stat !~ /(?: + \n.}\s*$| ^.DEFINE_$Ident\(\Q$name\E\)| ^.DECLARE_$Ident\(\Q$name\E\)| ^.LIST_HEAD\(\Q$name\E\)| - ^.$Type\s*\(\s*\*\s*\Q$name\E\s*\)\s*\(| - \b\Q$name\E(?:\s+$Attribute)?\s*(?:;|=|\[) + ^.(?:$Storage\s+)?$Type\s*\(\s*\*\s*\Q$name\E\s*\)\s*\(| + \b\Q$name\E(?:\s+$Attribute)*\s*(?:;|=|\[|\() )/x) { - WARN("EXPORT_SYMBOL(foo); should immediately follow its function/variable\n" . $herecurr); +#print "FOO A<$lines[$realline_next - 1]> stat<$stat> name<$name>\n"; + $suppress_export{$realline_next} = 2; + } else { + $suppress_export{$realline_next} = 1; } } + if (!defined $suppress_export{$linenr} && + $prevline =~ /^.\s*$/ && + ($line =~ /EXPORT_SYMBOL.*\((.*)\)/ || + $line =~ /EXPORT_UNUSED_SYMBOL.*\((.*)\)/)) { +#print "FOO B <$lines[$linenr - 1]>\n"; + $suppress_export{$linenr} = 2; + } + if (defined $suppress_export{$linenr} && + $suppress_export{$linenr} == 2) { + WARN("EXPORT_SYMBOL", + "EXPORT_SYMBOL(foo); should immediately follow its function/variable\n" . $herecurr); + } -# check for external initialisers. +# check for global initialisers. if ($line =~ /^.$Type\s*$Ident\s*(?:\s+$Modifier)*\s*=\s*(0|NULL|false)\s*;/) { - ERROR("do not initialise externals to 0 or NULL\n" . + ERROR("GLOBAL_INITIALISERS", + "do not initialise globals to 0 or NULL\n" . $herecurr); } # check for static initialisers. if ($line =~ /\bstatic\s.*=\s*(0|NULL|false)\s*;/) { - ERROR("do not initialise statics to 0 or NULL\n" . + ERROR("INITIALISED_STATIC", + "do not initialise statics to 0 or NULL\n" . $herecurr); } +# check for static const char * arrays. + if ($line =~ /\bstatic\s+const\s+char\s*\*\s*(\w+)\s*\[\s*\]\s*=\s*/) { + WARN("STATIC_CONST_CHAR_ARRAY", + "static const char * array should probably be static const char * const\n" . + $herecurr); + } + +# check for static char foo[] = "bar" declarations. + if ($line =~ /\bstatic\s+char\s+(\w+)\s*\[\s*\]\s*=\s*"/) { + WARN("STATIC_CONST_CHAR_ARRAY", + "static char array declaration should probably be static const char\n" . + $herecurr); + } + +# check for declarations of struct pci_device_id + if ($line =~ /\bstruct\s+pci_device_id\s+\w+\s*\[\s*\]\s*\=\s*\{/) { + WARN("DEFINE_PCI_DEVICE_TABLE", + "Use DEFINE_PCI_DEVICE_TABLE for struct pci_device_id\n" . $herecurr); + } + # check for new typedefs, only function parameters and sparse annotations # make sense. if ($line =~ /\btypedef\s/ && @@ -1654,13 +2301,15 @@ sub process { $line !~ /\btypedef\s+$Type\s+$Ident\s*\(/ && $line !~ /\b$typeTypedefs\b/ && $line !~ /\b__bitwise(?:__|)\b/) { - WARN("do not add new typedefs\n" . $herecurr); + WARN("NEW_TYPEDEFS", + "do not add new typedefs\n" . $herecurr); } # * goes on variable not on type # (char*[ const]) - if ($line =~ m{\($NonptrType(\s*(?:$Modifier\b\s*|\*\s*)+)\)}) { - my ($from, $to) = ($1, $1); + while ($line =~ m{(\($NonptrType(\s*(?:$Modifier\b\s*|\*\s*)+)\))}g) { + #print "AA<$1>\n"; + my ($from, $to) = ($2, $2); # Should start with a space. $to =~ s/^(\S)/ $1/; @@ -1672,10 +2321,13 @@ sub process { #print "from<$from> to<$to>\n"; if ($from ne $to) { - ERROR("\"(foo$from)\" should be \"(foo$to)\"\n" . $herecurr); + ERROR("POINTER_LOCATION", + "\"(foo$from)\" should be \"(foo$to)\"\n" . $herecurr); } - } elsif ($line =~ m{\b$NonptrType(\s*(?:$Modifier\b\s*|\*\s*)+)($Ident)}) { - my ($from, $to, $ident) = ($1, $1, $2); + } + while ($line =~ m{(\b$NonptrType(\s*(?:$Modifier\b\s*|\*\s*)+)($Ident))}g) { + #print "BB<$1>\n"; + my ($from, $to, $ident) = ($2, $2, $3); # Should start with a space. $to =~ s/^(\S)/ $1/; @@ -1689,7 +2341,8 @@ sub process { #print "from<$from> to<$to> ident<$ident>\n"; if ($from ne $to && $ident !~ /^$Modifier$/) { - ERROR("\"foo${from}bar\" should be \"foo${to}bar\"\n" . $herecurr); + ERROR("POINTER_LOCATION", + "\"foo${from}bar\" should be \"foo${to}bar\"\n" . $herecurr); } } @@ -1701,19 +2354,26 @@ sub process { # } if ($line =~ /\bLINUX_VERSION_CODE\b/) { - WARN("LINUX_VERSION_CODE should be avoided, code should be for the version to which it is merged\n" . $herecurr); + WARN("LINUX_VERSION_CODE", + "LINUX_VERSION_CODE should be avoided, code should be for the version to which it is merged\n" . $herecurr); + } + +# check for uses of printk_ratelimit + if ($line =~ /\bprintk_ratelimit\s*\(/) { + WARN("PRINTK_RATELIMITED", +"Prefer printk_ratelimited or pr__ratelimited to printk_ratelimit\n" . $herecurr); } # printk should use KERN_* levels. Note that follow on printk's on the # same line do not need a level, so we use the current block context # to try and find and validate the current printk. In summary the current -# printk includes all preceeding printk's which have no newline on the end. +# printk includes all preceding printk's which have no newline on the end. # we assume the first bad printk is the one to report. if ($line =~ /\bprintk\((?!KERN_)\s*"/) { my $ok = 0; for (my $ln = $linenr - 1; $ln >= $first_line; $ln--) { #print "CHECK<$lines[$ln - 1]\n"; - # we have a preceeding printk if it ends + # we have a preceding printk if it ends # with "\n" ignore it, else it is to blame if ($lines[$ln - 1] =~ m{\bprintk\(}) { if ($rawlines[$ln - 1] !~ m{\\n"}) { @@ -1723,7 +2383,8 @@ sub process { } } if ($ok == 0) { - WARN("printk() should include KERN_ facility level\n" . $herecurr); + WARN("PRINTK_WITHOUT_KERN_LEVEL", + "printk() should include KERN_ facility level\n" . $herecurr); } } @@ -1731,13 +2392,21 @@ sub process { # or if closed on same line if (($line=~/$Type\s*$Ident\(.*\).*\s{/) and !($line=~/\#\s*define.*do\s{/) and !($line=~/}/)) { - ERROR("open brace '{' following function declarations go on the next line\n" . $herecurr); + ERROR("OPEN_BRACE", + "open brace '{' following function declarations go on the next line\n" . $herecurr); } # open braces for enum, union and struct go on the same line. if ($line =~ /^.\s*{/ && $prevline =~ /^.\s*(?:typedef\s+)?(enum|union|struct)(?:\s+$Ident)?\s*$/) { - ERROR("open brace '{' following $1 go on the same line\n" . $hereprev); + ERROR("OPEN_BRACE", + "open brace '{' following $1 go on the same line\n" . $hereprev); + } + +# missing space after union, struct or enum definition + if ($line =~ /^.\s*(?:typedef\s+)?(enum|union|struct)(?:\s+$Ident)?(?:\s+$Ident)?[=\{]/) { + WARN("SPACING", + "missing space after $1 definition\n" . $herecurr); } # check for spacing round square brackets; allowed: @@ -1748,8 +2417,9 @@ sub process { my ($where, $prefix) = ($-[1], $1); if ($prefix !~ /$Type\s+$/ && ($where != 0 || $prefix !~ /^.\s+$/) && - $prefix !~ /{\s+$/) { - ERROR("space prohibited before open square bracket '['\n" . $herecurr); + $prefix !~ /[{,]\s+$/) { + ERROR("BRACKET_SPACE", + "space prohibited before open square bracket '['\n" . $herecurr); } } @@ -1780,7 +2450,8 @@ sub process { } elsif ($ctx =~ /$Type$/) { } else { - WARN("space prohibited between function name and open parenthesis '('\n" . $herecurr); + WARN("SPACING", + "space prohibited between function name and open parenthesis '('\n" . $herecurr); } } # Check operator spacing. @@ -1800,7 +2471,7 @@ sub process { for (my $n = 0; $n < $#elements; $n += 2) { $off += length($elements[$n]); - # Pick up the preceeding and succeeding characters. + # Pick up the preceding and succeeding characters. my $ca = substr($opline, 0, $off); my $cc = ''; if (length($opline) >= ($off + length($elements[$n + 1]))) { @@ -1854,7 +2525,8 @@ sub process { } elsif ($op eq ';') { if ($ctx !~ /.x[WEBC]/ && $cc !~ /^\\/ && $cc !~ /^;/) { - ERROR("space required after that '$op' $at\n" . $hereptr); + ERROR("SPACING", + "space required after that '$op' $at\n" . $hereptr); } # // is a comment @@ -1865,13 +2537,15 @@ sub process { # : when part of a bitfield } elsif ($op eq '->' || $opv eq ':B') { if ($ctx =~ /Wx.|.xW/) { - ERROR("spaces prohibited around that '$op' $at\n" . $hereptr); + ERROR("SPACING", + "spaces prohibited around that '$op' $at\n" . $hereptr); } # , must have a space on the right. } elsif ($op eq ',') { if ($ctx !~ /.x[WEC]/ && $cc !~ /^}/) { - ERROR("space required after that '$op' $at\n" . $hereptr); + ERROR("SPACING", + "space required after that '$op' $at\n" . $hereptr); } # '*' as part of a type definition -- reported already. @@ -1885,26 +2559,31 @@ sub process { $opv eq '*U' || $opv eq '-U' || $opv eq '&U' || $opv eq '&&U') { if ($ctx !~ /[WEBC]x./ && $ca !~ /(?:\)|!|~|\*|-|\&|\||\+\+|\-\-|\{)$/) { - ERROR("space required before that '$op' $at\n" . $hereptr); + ERROR("SPACING", + "space required before that '$op' $at\n" . $hereptr); } if ($op eq '*' && $cc =~/\s*$Modifier\b/) { # A unary '*' may be const } elsif ($ctx =~ /.xW/) { - ERROR("Aspace prohibited after that '$op' $at\n" . $hereptr); + ERROR("SPACING", + "space prohibited after that '$op' $at\n" . $hereptr); } # unary ++ and unary -- are allowed no space on one side. } elsif ($op eq '++' or $op eq '--') { if ($ctx !~ /[WEOBC]x[^W]/ && $ctx !~ /[^W]x[WOBEC]/) { - ERROR("space required one side of that '$op' $at\n" . $hereptr); + ERROR("SPACING", + "space required one side of that '$op' $at\n" . $hereptr); } if ($ctx =~ /Wx[BE]/ || ($ctx =~ /Wx./ && $cc =~ /^;/)) { - ERROR("space prohibited before that '$op' $at\n" . $hereptr); + ERROR("SPACING", + "space prohibited before that '$op' $at\n" . $hereptr); } if ($ctx =~ /ExW/) { - ERROR("space prohibited after that '$op' $at\n" . $hereptr); + ERROR("SPACING", + "space prohibited after that '$op' $at\n" . $hereptr); } @@ -1916,7 +2595,8 @@ sub process { $op eq '%') { if ($ctx =~ /Wx[^WCE]|[^WCE]xW/) { - ERROR("need consistent spacing around '$op' $at\n" . + ERROR("SPACING", + "need consistent spacing around '$op' $at\n" . $hereptr); } @@ -1924,7 +2604,8 @@ sub process { # terminating a case value or a label. } elsif ($opv eq ':C' || $opv eq ':L') { if ($ctx =~ /Wx./) { - ERROR("space prohibited before that '$op' $at\n" . $hereptr); + ERROR("SPACING", + "space prohibited before that '$op' $at\n" . $hereptr); } # All the others need spaces both sides. @@ -1947,7 +2628,8 @@ sub process { } if ($ok == 0) { - ERROR("spaces required around that '$op' $at\n" . $hereptr); + ERROR("SPACING", + "spaces required around that '$op' $at\n" . $hereptr); } } $off += length($elements[$n + 1]); @@ -1956,7 +2638,8 @@ sub process { # check for multiple assignments if ($line =~ /^.\s*$Lval\s*=\s*$Lval\s*=(?!=)/) { - CHK("multiple assignments should be avoided\n" . $herecurr); + CHK("MULTIPLE_ASSIGNMENTS", + "multiple assignments should be avoided\n" . $herecurr); } ## # check for multiple declarations, allowing for a function declaration @@ -1970,45 +2653,53 @@ sub process { ## while ($ln =~ s/\([^\(\)]*\)//g) { ## } ## if ($ln =~ /,/) { -## WARN("declaring multiple variables together should be avoided\n" . $herecurr); +## WARN("MULTIPLE_DECLARATION", +## "declaring multiple variables together should be avoided\n" . $herecurr); ## } ## } #need space before brace following if, while, etc if (($line =~ /\(.*\){/ && $line !~ /\($Type\){/) || $line =~ /do{/) { - ERROR("space required before the open brace '{'\n" . $herecurr); + ERROR("SPACING", + "space required before the open brace '{'\n" . $herecurr); } # closing brace should have a space following it when it has anything # on the line if ($line =~ /}(?!(?:,|;|\)))\S/) { - ERROR("space required after that close brace '}'\n" . $herecurr); + ERROR("SPACING", + "space required after that close brace '}'\n" . $herecurr); } # check spacing on square brackets if ($line =~ /\[\s/ && $line !~ /\[\s*$/) { - ERROR("space prohibited after that open square bracket '['\n" . $herecurr); + ERROR("SPACING", + "space prohibited after that open square bracket '['\n" . $herecurr); } if ($line =~ /\s\]/) { - ERROR("space prohibited before that close square bracket ']'\n" . $herecurr); + ERROR("SPACING", + "space prohibited before that close square bracket ']'\n" . $herecurr); } # check spacing on parentheses if ($line =~ /\(\s/ && $line !~ /\(\s*(?:\\)?$/ && $line !~ /for\s*\(\s+;/) { - ERROR("space prohibited after that open parenthesis '('\n" . $herecurr); + ERROR("SPACING", + "space prohibited after that open parenthesis '('\n" . $herecurr); } if ($line =~ /(\s+)\)/ && $line !~ /^.\s*\)/ && $line !~ /for\s*\(.*;\s+\)/ && $line !~ /:\s+\)/) { - ERROR("space prohibited before that close parenthesis ')'\n" . $herecurr); + ERROR("SPACING", + "space prohibited before that close parenthesis ')'\n" . $herecurr); } #goto labels aren't indented, allow a single space however if ($line=~/^.\s+[A-Za-z\d_]+:(?![0-9]+)/ and !($line=~/^. [A-Za-z\d_]+:/) and !($line=~/^.\s+default:/)) { - WARN("labels should not be indented\n" . $herecurr); + WARN("INDENTED_LABEL", + "labels should not be indented\n" . $herecurr); } # Return is not a function. @@ -2017,30 +2708,44 @@ sub process { my $value = $2; # Flatten any parentheses - $value =~ s/\)\(/\) \(/g; - while ($value =~ s/\[[^\{\}]*\]/1/ || + $value =~ s/\(/ \(/g; + $value =~ s/\)/\) /g; + while ($value =~ s/\[[^\[\]]*\]/1/ || $value !~ /(?:$Ident|-?$Constant)\s* $Compare\s* (?:$Ident|-?$Constant)/x && $value =~ s/\([^\(\)]*\)/1/) { } - - if ($value =~ /^(?:$Ident|-?$Constant)$/) { - ERROR("return is not a function, parentheses are not required\n" . $herecurr); +#print "value<$value>\n"; + if ($value =~ /^\s*(?:$Ident|-?$Constant)\s*$/) { + ERROR("RETURN_PARENTHESES", + "return is not a function, parentheses are not required\n" . $herecurr); } elsif ($spacing !~ /\s+/) { - ERROR("space required before the open parenthesis '('\n" . $herecurr); + ERROR("SPACING", + "space required before the open parenthesis '('\n" . $herecurr); + } + } +# Return of what appears to be an errno should normally be -'ve + if ($line =~ /^.\s*return\s*(E[A-Z]*)\s*;/) { + my $name = $1; + if ($name ne 'EOF' && $name ne 'ERROR') { + WARN("USE_NEGATIVE_ERRNO", + "return of an errno should typically be -ve (return -$1)\n" . $herecurr); } } # Need a space before open parenthesis after if, while etc if ($line=~/\b(if|while|for|switch)\(/) { - ERROR("space required before the open parenthesis '('\n" . $herecurr); + ERROR("SPACING", "space required before the open parenthesis '('\n" . $herecurr); } # Check for illegal assignment in if conditional -- and check for trailing # statements after the conditional. if ($line =~ /do\s*(?!{)/) { + ($stat, $cond, $line_nr_next, $remain_next, $off_next) = + ctx_statement_block($linenr, $realcnt, 0) + if (!defined $stat); my ($stat_next) = ctx_statement_block($line_nr_next, $remain_next, $off_next); $stat_next =~ s/\n./\n /g; @@ -2063,7 +2768,8 @@ sub process { my ($s, $c) = ($stat, $cond); if ($c =~ /\bif\s*\(.*[^<>!=]=[^=].*/s) { - ERROR("do not use assignment in if condition\n" . $herecurr); + ERROR("ASSIGN_IN_IF", + "do not use assignment in if condition\n" . $herecurr); } # Find out what is on the end of the line after the @@ -2077,13 +2783,16 @@ sub process { # Find out how long the conditional actually is. my @newlines = ($c =~ /\n/gs); my $cond_lines = 1 + $#newlines; + my $stat_real = ''; - my $stat_real = raw_line($linenr, $cond_lines); + $stat_real = raw_line($linenr, $cond_lines) + . "\n" if ($cond_lines); if (defined($stat_real) && $cond_lines > 1) { $stat_real = "[...]\n$stat_real"; } - ERROR("trailing statements should be on next line\n" . $herecurr . $stat_real); + ERROR("TRAILING_STATEMENTS", + "trailing statements should be on next line\n" . $herecurr . $stat_real); } } @@ -2099,7 +2808,8 @@ sub process { (?:\&\&|\|\||\)|\]) )/x) { - WARN("boolean test with hexadecimal, perhaps just 1 \& or \|?\n" . $herecurr); + WARN("HEXADECIMAL_BOOLEAN_TEST", + "boolean test with hexadecimal, perhaps just 1 \& or \|?\n" . $herecurr); } # if and else should not have general statements after it @@ -2107,12 +2817,14 @@ sub process { my $s = $1; $s =~ s/$;//g; # Remove any comments if ($s !~ /^\s*(?:\sif|(?:{|)\s*\\?\s*$)/) { - ERROR("trailing statements should be on next line\n" . $herecurr); + ERROR("TRAILING_STATEMENTS", + "trailing statements should be on next line\n" . $herecurr); } } # if should not continue a brace if ($line =~ /}\s*if\b/) { - ERROR("trailing statements should be on next line\n" . + ERROR("TRAILING_STATEMENTS", + "trailing statements should be on next line\n" . $herecurr); } # case and default should not have general statements after them @@ -2122,14 +2834,16 @@ sub process { \s*return\s+ )/xg) { - ERROR("trailing statements should be on next line\n" . $herecurr); + ERROR("TRAILING_STATEMENTS", + "trailing statements should be on next line\n" . $herecurr); } # Check for }else {, these must be at the same # indent level to be relevant to each other. if ($prevline=~/}\s*$/ and $line=~/^.\s*else\s*/ and $previndent == $indent) { - ERROR("else should follow close brace '}'\n" . $hereprev); + ERROR("ELSE_AFTER_BRACE", + "else should follow close brace '}'\n" . $hereprev); } if ($prevline=~/}\s*$/ and $line=~/^.\s*while\s*/ and @@ -2142,7 +2856,8 @@ sub process { $s =~ s/\n.*//g; if ($s =~ /^\s*;/) { - ERROR("while should follow close brace '}'\n" . $hereprev); + ERROR("WHILE_AFTER_BRACE", + "while should follow close brace '}'\n" . $hereprev); } } @@ -2155,7 +2870,8 @@ sub process { #no spaces allowed after \ in define if ($line=~/\#\s*define.*\\\s$/) { - WARN("Whitepspace after \\ makes next lines useless\n" . $herecurr); + WARN("WHITESPACE_AFTER_LINE_CONTINUATION", + "Whitepspace after \\ makes next lines useless\n" . $herecurr); } #warn if is #included and is available (uses RAW line) @@ -2164,12 +2880,14 @@ sub process { my $checkfile = "include/linux/$file"; if (-f "$root/$checkfile" && $realfile ne $checkfile && - $1 ne 'irq') + $1 !~ /$allowed_asm_includes/) { if ($realfile =~ m{^arch/}) { - CHK("Consider using #include instead of \n" . $herecurr); + CHK("ARCH_INCLUDE_LINUX", + "Consider using #include instead of \n" . $herecurr); } else { - WARN("Use #include instead of \n" . $herecurr); + WARN("INCLUDE_LINUX", + "Use #include instead of \n" . $herecurr); } } } @@ -2183,47 +2901,13 @@ sub process { my $cnt = $realcnt; my ($off, $dstat, $dcond, $rest); my $ctx = ''; - - my $args = defined($1); - - # Find the end of the macro and limit our statement - # search to that. - while ($cnt > 0 && defined $lines[$ln - 1] && - $lines[$ln - 1] =~ /^(?:-|..*\\$)/) - { - $ctx .= $rawlines[$ln - 1] . "\n"; - $cnt-- if ($lines[$ln - 1] !~ /^-/); - $ln++; - } - $ctx .= $rawlines[$ln - 1]; - ($dstat, $dcond, $ln, $cnt, $off) = - ctx_statement_block($linenr, $ln - $linenr + 1, 0); + ctx_statement_block($linenr, $realcnt, 0); + $ctx = $dstat; #print "dstat<$dstat> dcond<$dcond> cnt<$cnt> off<$off>\n"; #print "LINE<$lines[$ln-1]> len<" . length($lines[$ln-1]) . "\n"; - # Extract the remainder of the define (if any) and - # rip off surrounding spaces, and trailing \'s. - $rest = ''; - while ($off != 0 || ($cnt > 0 && $rest =~ /\\\s*$/)) { - #print "ADDING cnt<$cnt> $off <" . substr($lines[$ln - 1], $off) . "> rest<$rest>\n"; - if ($off != 0 || $lines[$ln - 1] !~ /^-/) { - $rest .= substr($lines[$ln - 1], $off) . "\n"; - $cnt--; - } - $ln++; - $off = 0; - } - $rest =~ s/\\\n.//g; - $rest =~ s/^\s*//s; - $rest =~ s/\s*$//s; - - # Clean up the original statement. - if ($args) { - substr($dstat, 0, length($dcond), ''); - } else { - $dstat =~ s/^.\s*\#\s*define\s+$Ident\s*//; - } + $dstat =~ s/^.\s*\#\s*define\s+$Ident(?:\([^\)]*\))?\s*//; $dstat =~ s/$;//g; $dstat =~ s/\\\n.//g; $dstat =~ s/^\s*//s; @@ -2232,7 +2916,13 @@ sub process { # Flatten any parentheses and braces while ($dstat =~ s/\([^\(\)]*\)/1/ || $dstat =~ s/\{[^\{\}]*\}/1/ || - $dstat =~ s/\[[^\{\}]*\]/1/) + $dstat =~ s/\[[^\[\]]*\]/1/) + { + } + + # Flatten any obvious string concatentation. + while ($dstat =~ s/("X*")\s*$Ident/$1/ || + $dstat =~ s/$Ident\s*("X*")/$1/) { } @@ -2243,24 +2933,39 @@ sub process { DECLARE_PER_CPU| DEFINE_PER_CPU| __typeof__\(| - \.$Ident\s*=\s* + union| + struct| + \.$Ident\s*=\s*| + ^\"|\"$ }x; - #print "REST<$rest> dstat<$dstat>\n"; - if ($rest ne '') { - if ($rest !~ /while\s*\(/ && - $dstat !~ /$exceptions/) - { - ERROR("Macros with multiple statements should be enclosed in a do - while loop\n" . "$here\n$ctx\n"); + #print "REST<$rest> dstat<$dstat> ctx<$ctx>\n"; + if ($dstat ne '' && + $dstat !~ /^(?:$Ident|-?$Constant),$/ && # 10, // foo(), + $dstat !~ /^(?:$Ident|-?$Constant);$/ && # foo(); + $dstat !~ /^[!~-]?(?:$Ident|$Constant)$/ && # 10 // foo() // !foo // ~foo // -foo + $dstat !~ /^'X'$/ && # character constants + $dstat !~ /$exceptions/ && + $dstat !~ /^\.$Ident\s*=/ && # .foo = + $dstat !~ /^do\s*$Constant\s*while\s*$Constant;?$/ && # do {...} while (...); // do {...} while (...) + $dstat !~ /^for\s*$Constant$/ && # for (...) + $dstat !~ /^for\s*$Constant\s+(?:$Ident|-?$Constant)$/ && # for (...) bar() + $dstat !~ /^do\s*{/ && # do {... + $dstat !~ /^\({/) # ({... + { + $ctx =~ s/\n*$//; + my $herectx = $here . "\n"; + my $cnt = statement_rawlines($ctx); + + for (my $n = 0; $n < $cnt; $n++) { + $herectx .= raw_line($linenr, $n) . "\n"; } - } elsif ($ctx !~ /;/) { - if ($dstat ne '' && - $dstat !~ /^(?:$Ident|-?$Constant)$/ && - $dstat !~ /$exceptions/ && - $dstat !~ /^\.$Ident\s*=/ && - $dstat =~ /$Operators/) - { - ERROR("Macros with complex values should be enclosed in parenthesis\n" . "$here\n$ctx\n"); + if ($dstat =~ /;/) { + ERROR("MULTISTATEMENT_MACRO_USE_DO_WHILE", + "Macros with multiple statements should be enclosed in a do - while loop\n" . "$herectx"); + } else { + ERROR("COMPLEX_MACRO", + "Macros with complex values should be enclosed in parenthesis\n" . "$herectx"); } } } @@ -2271,7 +2976,8 @@ sub process { # ALIGN(...) # VMLINUX_SYMBOL(...) if ($realfile eq 'vmlinux.lds.h' && $line =~ /(?:(?:^|\s)$Ident\s*=|=\s*$Ident(?:\s|$))/) { - WARN("vmlinux.lds.h needs VMLINUX_SYMBOL() around C-visible symbols\n" . $herecurr); + WARN("MISSING_VMLINUX_SYMBOL", + "vmlinux.lds.h needs VMLINUX_SYMBOL() around C-visible symbols\n" . $herecurr); } # check for redundant bracing round if etc @@ -2281,7 +2987,8 @@ sub process { #print "chunks<$#chunks> linenr<$linenr> endln<$endln> level<$level>\n"; #print "APW: <<$chunks[1][0]>><<$chunks[1][1]>>\n"; if ($#chunks > 0 && $level == 0) { - my $allowed = 0; + my @allowed = (); + my $allow = 0; my $seen = 0; my $herectx = $here . "\n"; my $ln = $linenr - 1; @@ -2292,6 +2999,7 @@ sub process { my ($whitespace) = ($cond =~ /^((?:\s*\n[+-])*\s*)/s); my $offset = statement_rawlines($whitespace) - 1; + $allowed[$allow] = 0; #print "COND<$cond> whitespace<$whitespace> offset<$offset>\n"; # We have looked at and allowed this specific line. @@ -2304,22 +3012,34 @@ sub process { $seen++ if ($block =~ /^\s*{/); - #print "cond<$cond> block<$block> allowed<$allowed>\n"; + #print "cond<$cond> block<$block> allowed<$allowed[$allow]>\n"; if (statement_lines($cond) > 1) { #print "APW: ALLOWED: cond<$cond>\n"; - $allowed = 1; + $allowed[$allow] = 1; } if ($block =~/\b(?:if|for|while)\b/) { #print "APW: ALLOWED: block<$block>\n"; - $allowed = 1; + $allowed[$allow] = 1; } if (statement_block_size($block) > 1) { #print "APW: ALLOWED: lines block<$block>\n"; - $allowed = 1; + $allowed[$allow] = 1; } + $allow++; } - if ($seen && !$allowed) { - WARN("braces {} are not necessary for any arm of this statement\n" . $herectx); + if ($seen) { + my $sum_allowed = 0; + foreach (@allowed) { + $sum_allowed += $_; + } + if ($sum_allowed == 0) { + WARN("BRACES", + "braces {} are not necessary for any arm of this statement\n" . $herectx); + } elsif ($sum_allowed != $allow && + $seen != $allow) { + CHK("BRACES", + "braces {} should be used on all arms of this statement\n" . $herectx); + } } } } @@ -2366,45 +3086,45 @@ sub process { } } if ($level == 0 && $block =~ /^\s*\{/ && !$allowed) { - my $herectx = $here . "\n";; + my $herectx = $here . "\n"; my $cnt = statement_rawlines($block); for (my $n = 0; $n < $cnt; $n++) { - $herectx .= raw_line($linenr, $n) . "\n";; + $herectx .= raw_line($linenr, $n) . "\n"; } - WARN("braces {} are not necessary for single statement blocks\n" . $herectx); + WARN("BRACES", + "braces {} are not necessary for single statement blocks\n" . $herectx); } } # don't include deprecated include files (uses RAW line) for my $inc (@dep_includes) { if ($rawline =~ m@^.\s*\#\s*include\s*\<$inc>@) { - ERROR("Don't use <$inc>: see Documentation/feature-removal-schedule.txt\n" . $herecurr); + ERROR("DEPRECATED_INCLUDE", + "Don't use <$inc>: see Documentation/feature-removal-schedule.txt\n" . $herecurr); } } # don't use deprecated functions for my $func (@dep_functions) { if ($line =~ /\b$func\b/) { - ERROR("Don't use $func(): see Documentation/feature-removal-schedule.txt\n" . $herecurr); + ERROR("DEPRECATED_FUNCTION", + "Don't use $func(): see Documentation/feature-removal-schedule.txt\n" . $herecurr); } } # no volatiles please my $asm_volatile = qr{\b(__asm__|asm)\s+(__volatile__|volatile)\b}; if ($line =~ /\bvolatile\b/ && $line !~ /$asm_volatile/) { - WARN("Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt\n" . $herecurr); - } - -# SPIN_LOCK_UNLOCKED & RW_LOCK_UNLOCKED are deprecated - if ($line =~ /\b(SPIN_LOCK_UNLOCKED|RW_LOCK_UNLOCKED)/) { - ERROR("Use of $1 is deprecated: see Documentation/spinlocks.txt\n" . $herecurr); + WARN("VOLATILE", + "Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt\n" . $herecurr); } # warn about #if 0 if ($line =~ /^.\s*\#\s*if\s+0\b/) { - CHK("if this code is redundant consider removing it\n" . + CHK("REDUNDANT_CODE", + "if this code is redundant consider removing it\n" . $herecurr); } @@ -2412,14 +3132,33 @@ sub process { if ($prevline =~ /\bif\s*\(([^\)]*)\)/) { my $expr = $1; if ($line =~ /\bkfree\(\Q$expr\E\);/) { - WARN("kfree(NULL) is safe this check is probably not required\n" . $hereprev); + WARN("NEEDLESS_KFREE", + "kfree(NULL) is safe this check is probably not required\n" . $hereprev); } } # check for needless usb_free_urb() checks if ($prevline =~ /\bif\s*\(([^\)]*)\)/) { my $expr = $1; if ($line =~ /\busb_free_urb\(\Q$expr\E\);/) { - WARN("usb_free_urb(NULL) is safe this check is probably not required\n" . $hereprev); + WARN("NEEDLESS_USB_FREE_URB", + "usb_free_urb(NULL) is safe this check is probably not required\n" . $hereprev); + } + } + +# prefer usleep_range over udelay + if ($line =~ /\budelay\s*\(\s*(\w+)\s*\)/) { + # ignore udelay's < 10, however + if (! (($1 =~ /(\d+)/) && ($1 < 10)) ) { + CHK("USLEEP_RANGE", + "usleep_range is preferred over udelay; see Documentation/timers/timers-howto.txt\n" . $line); + } + } + +# warn about unexpectedly long msleep's + if ($line =~ /\bmsleep\s*\((\d+)\);/) { + if ($1 < 20) { + WARN("MSLEEP", + "msleep < 20ms can sleep for up to 20ms; see Documentation/timers/timers-howto.txt\n" . $line); } } @@ -2432,7 +3171,8 @@ sub process { # warn about spacing in #ifdefs if ($line =~ /^.\s*\#\s*(ifdef|ifndef|elif)\s\s+/) { - ERROR("exactly one space required after that #$1\n" . $herecurr); + ERROR("SPACING", + "exactly one space required after that #$1\n" . $herecurr); } # check for spinlock_t definitions without a comment. @@ -2440,30 +3180,119 @@ sub process { $line =~ /^.\s*(DEFINE_MUTEX)\s*\(/) { my $which = $1; if (!ctx_has_comment($first_line, $linenr)) { - CHK("$1 definition without comment\n" . $herecurr); + CHK("UNCOMMENTED_DEFINITION", + "$1 definition without comment\n" . $herecurr); } } # check for memory barriers without a comment. if ($line =~ /\b(mb|rmb|wmb|read_barrier_depends|smp_mb|smp_rmb|smp_wmb|smp_read_barrier_depends)\(/) { if (!ctx_has_comment($first_line, $linenr)) { - CHK("memory barrier without comment\n" . $herecurr); + CHK("MEMORY_BARRIER", + "memory barrier without comment\n" . $herecurr); } } # check of hardware specific defines if ($line =~ m@^.\s*\#\s*if.*\b(__i386__|__powerpc64__|__sun__|__s390x__)\b@ && $realfile !~ m@include/asm-@) { - CHK("architecture specific defines should be avoided\n" . $herecurr); + CHK("ARCH_DEFINES", + "architecture specific defines should be avoided\n" . $herecurr); + } + +# Check that the storage class is at the beginning of a declaration + if ($line =~ /\b$Storage\b/ && $line !~ /^.\s*$Storage\b/) { + WARN("STORAGE_CLASS", + "storage class should be at the beginning of the declaration\n" . $herecurr) } # check the location of the inline attribute, that it is between # storage class and type. if ($line =~ /\b$Type\s+$Inline\b/ || $line =~ /\b$Inline\s+$Storage\b/) { - ERROR("inline keyword should sit between storage class and type\n" . $herecurr); + ERROR("INLINE_LOCATION", + "inline keyword should sit between storage class and type\n" . $herecurr); } # Check for __inline__ and __inline, prefer inline if ($line =~ /\b(__inline__|__inline)\b/) { - WARN("plain inline is preferred over $1\n" . $herecurr); + WARN("INLINE", + "plain inline is preferred over $1\n" . $herecurr); + } + +# Check for __attribute__ packed, prefer __packed + if ($line =~ /\b__attribute__\s*\(\s*\(.*\bpacked\b/) { + WARN("PREFER_PACKED", + "__packed is preferred over __attribute__((packed))\n" . $herecurr); + } + +# Check for __attribute__ aligned, prefer __aligned + if ($line =~ /\b__attribute__\s*\(\s*\(.*aligned/) { + WARN("PREFER_ALIGNED", + "__aligned(size) is preferred over __attribute__((aligned(size)))\n" . $herecurr); + } + +# Check for __attribute__ format(printf, prefer __printf + if ($line =~ /\b__attribute__\s*\(\s*\(\s*format\s*\(\s*printf/) { + WARN("PREFER_PRINTF", + "__printf(string-index, first-to-check) is preferred over __attribute__((format(printf, string-index, first-to-check)))\n" . $herecurr); + } + +# Check for __attribute__ format(scanf, prefer __scanf + if ($line =~ /\b__attribute__\s*\(\s*\(\s*format\s*\(\s*scanf\b/) { + WARN("PREFER_SCANF", + "__scanf(string-index, first-to-check) is preferred over __attribute__((format(scanf, string-index, first-to-check)))\n" . $herecurr); + } + +# check for sizeof(&) + if ($line =~ /\bsizeof\s*\(\s*\&/) { + WARN("SIZEOF_ADDRESS", + "sizeof(& should be avoided\n" . $herecurr); + } + +# check for line continuations in quoted strings with odd counts of " + if ($rawline =~ /\\$/ && $rawline =~ tr/"/"/ % 2) { + WARN("LINE_CONTINUATIONS", + "Avoid line continuations in quoted strings\n" . $herecurr); + } + +# Check for misused memsets + if ($^V && $^V ge 5.10.0 && + defined $stat && + $stat =~ /^\+(?:.*?)\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*$FuncArg\s*\)/s) { + + my $ms_addr = $2; + my $ms_val = $7; + my $ms_size = $12; + + if ($ms_size =~ /^(0x|)0$/i) { + ERROR("MEMSET", + "memset to 0's uses 0 as the 2nd argument, not the 3rd\n" . "$here\n$stat\n"); + } elsif ($ms_size =~ /^(0x|)1$/i) { + WARN("MEMSET", + "single byte memset is suspicious. Swapped 2nd/3rd argument?\n" . "$here\n$stat\n"); + } + } + +# typecasts on min/max could be min_t/max_t + if ($^V && $^V ge 5.10.0 && + defined $stat && + $stat =~ /^\+(?:.*?)\b(min|max)\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\)/) { + if (defined $2 || defined $7) { + my $call = $1; + my $cast1 = deparenthesize($2); + my $arg1 = $3; + my $cast2 = deparenthesize($7); + my $arg2 = $8; + my $cast; + + if ($cast1 ne "" && $cast2 ne "" && $cast1 ne $cast2) { + $cast = "$cast1 or $cast2"; + } elsif ($cast1 ne "") { + $cast = $cast1; + } else { + $cast = $cast2; + } + WARN("MINMAX", + "$call() should probably be ${call}_t($cast, $arg1, $arg2)\n" . "$here\n$stat\n"); + } } # check for new externs in .c files. @@ -2480,17 +3309,20 @@ sub process { if ($s =~ /^\s*;/ && $function_name ne 'uninitialized_var') { - WARN("externs should be avoided in .c files\n" . $herecurr); + WARN("AVOID_EXTERNS", + "externs should be avoided in .c files\n" . $herecurr); } if ($paren_space =~ /\n/) { - WARN("arguments for function declarations should follow identifier\n" . $herecurr); + WARN("FUNCTION_ARGUMENTS", + "arguments for function declarations should follow identifier\n" . $herecurr); } } elsif ($realfile =~ /\.c$/ && defined $stat && $stat =~ /^.\s*extern\s+/) { - WARN("externs should be avoided in .c files\n" . $herecurr); + WARN("AVOID_EXTERNS", + "externs should be avoided in .c files\n" . $herecurr); } # checks for new __setup's @@ -2498,40 +3330,95 @@ sub process { my $name = $1; if (!grep(/$name/, @setup_docs)) { - CHK("__setup appears un-documented -- check Documentation/kernel-parameters.txt\n" . $herecurr); + CHK("UNDOCUMENTED_SETUP", + "__setup appears un-documented -- check Documentation/kernel-parameters.txt\n" . $herecurr); } } # check for pointless casting of kmalloc return - if ($line =~ /\*\s*\)\s*k[czm]alloc\b/) { - WARN("unnecessary cast may hide bugs, see http://c-faq.com/malloc/mallocnocast.html\n" . $herecurr); + if ($line =~ /\*\s*\)\s*[kv][czm]alloc(_node){0,1}\b/) { + WARN("UNNECESSARY_CASTS", + "unnecessary cast may hide bugs, see http://c-faq.com/malloc/mallocnocast.html\n" . $herecurr); + } + +# check for multiple semicolons + if ($line =~ /;\s*;\s*$/) { + WARN("ONE_SEMICOLON", + "Statements terminations use 1 semicolon\n" . $herecurr); } # check for gcc specific __FUNCTION__ if ($line =~ /__FUNCTION__/) { - WARN("__func__ should be used instead of gcc specific __FUNCTION__\n" . $herecurr); + WARN("USE_FUNC", + "__func__ should be used instead of gcc specific __FUNCTION__\n" . $herecurr); } -# check for semaphores used as mutexes - if ($line =~ /^.\s*(DECLARE_MUTEX|init_MUTEX)\s*\(/) { - WARN("mutexes are preferred for single holder semaphores\n" . $herecurr); +# check for use of yield() + if ($line =~ /\byield\s*\(\s*\)/) { + WARN("YIELD", + "Using yield() is generally wrong. See yield() kernel-doc (sched/core.c)\n" . $herecurr); } -# check for semaphores used as mutexes - if ($line =~ /^.\s*init_MUTEX_LOCKED\s*\(/) { - WARN("consider using a completion\n" . $herecurr); + +# check for semaphores initialized locked + if ($line =~ /^.\s*sema_init.+,\W?0\W?\)/) { + WARN("CONSIDER_COMPLETION", + "consider using a completion\n" . $herecurr); } -# recommend strict_strto* over simple_strto* - if ($line =~ /\bsimple_(strto.*?)\s*\(/) { - WARN("consider using strict_$1 in preference to simple_$1\n" . $herecurr); + +# recommend kstrto* over simple_strto* and strict_strto* + if ($line =~ /\b((simple|strict)_(strto(l|ll|ul|ull)))\s*\(/) { + WARN("CONSIDER_KSTRTO", + "$1 is obsolete, use k$3 instead\n" . $herecurr); } + # check for __initcall(), use device_initcall() explicitly please if ($line =~ /^.\s*__initcall\s*\(/) { - WARN("please use device_initcall() instead of __initcall()\n" . $herecurr); + WARN("USE_DEVICE_INITCALL", + "please use device_initcall() instead of __initcall()\n" . $herecurr); } -# check for struct file_operations, ensure they are const. + +# check for various ops structs, ensure they are const. + my $struct_ops = qr{acpi_dock_ops| + address_space_operations| + backlight_ops| + block_device_operations| + dentry_operations| + dev_pm_ops| + dma_map_ops| + extent_io_ops| + file_lock_operations| + file_operations| + hv_ops| + ide_dma_ops| + intel_dvo_dev_ops| + item_operations| + iwl_ops| + kgdb_arch| + kgdb_io| + kset_uevent_ops| + lock_manager_operations| + microcode_ops| + mtrr_ops| + neigh_ops| + nlmsvc_binding| + pci_raw_ops| + pipe_buf_operations| + platform_hibernation_ops| + platform_suspend_ops| + proto_ops| + rpc_pipe_ops| + seq_operations| + snd_ac97_build_ops| + soc_pcmcia_socket_ops| + stacktrace_ops| + sysfs_ops| + tty_operations| + usb_mon_operations| + wd_ops}x; if ($line !~ /\bconst\b/ && - $line =~ /\bstruct\s+(file_operations|seq_operations)\b/) { - WARN("struct $1 should normally be const\n" . + $line =~ /\bstruct\s+($struct_ops)\b/) { + WARN("CONST_STRUCT", + "struct $1 should normally be const\n" . $herecurr); } @@ -2544,7 +3431,8 @@ sub process { $line !~ /\[[^\]]*\.\.\.[^\]]*NR_CPUS[^\]]*\]/ && $line !~ /\[[^\]]*NR_CPUS[^\]]*\.\.\.[^\]]*\]/) { - WARN("usage of NR_CPUS is often wrong - consider using cpu_possible(), num_possible_cpus(), for_each_possible_cpu(), etc\n" . $herecurr); + WARN("NR_CPUS", + "usage of NR_CPUS is often wrong - consider using cpu_possible(), num_possible_cpus(), for_each_possible_cpu(), etc\n" . $herecurr); } # check for %L{u,d,i} in strings @@ -2553,7 +3441,8 @@ sub process { $string = substr($rawline, $-[1], $+[1] - $-[1]); $string =~ s/%%/__/g; if ($string =~ /(?mutex.\n" . $herecurr); + } + } + + if ($line =~ /debugfs_create_file.*S_IWUGO/ || + $line =~ /DEVICE_ATTR.*S_IWUGO/ ) { + WARN("EXPORTED_WORLD_WRITABLE", + "Exporting world writable files is usually an error. Consider more restrictive permissions.\n" . $herecurr); + } } # If we have no input at all, then there is nothing to report on @@ -2587,10 +3495,12 @@ sub process { } if (!$is_patch) { - ERROR("Does not appear to be a unified-diff format patch\n"); + ERROR("NOT_UNIFIED_DIFF", + "Does not appear to be a unified-diff format patch\n"); } if ($is_patch && $chk_signoff && $signoff == 0) { - ERROR("Missing Signed-off-by: line(s)\n"); + ERROR("MISSING_SIGN_OFF", + "Missing Signed-off-by: line(s)\n"); } print report_dump(); @@ -2602,13 +3512,40 @@ sub process { print "\n" if ($quiet == 0); } + if ($quiet == 0) { + + if ($^V lt 5.10.0) { + print("NOTE: perl $^V is not modern enough to detect all possible issues.\n"); + print("An upgrade to at least perl v5.10.0 is suggested.\n\n"); + } + + # If there were whitespace errors which cleanpatch can fix + # then suggest that. + if ($rpt_cleaners) { + print "NOTE: whitespace errors detected, you may wish to use scripts/cleanpatch or\n"; + print " scripts/cleanfile\n\n"; + $rpt_cleaners = 0; + } + } + + if ($quiet == 0 && keys %ignore_type) { + print "NOTE: Ignored message types:"; + foreach my $ignore (sort keys %ignore_type) { + print " $ignore"; + } + print "\n\n"; + } + if ($clean == 1 && $quiet == 0) { print "$vname has no obvious style problems and is ready for submission.\n" } if ($clean == 0 && $quiet == 0) { - print "$vname has style problems, please review. If any of these errors\n"; - print "are false positives report them to the maintainer, see\n"; - print "CHECKPATCH in MAINTAINERS.\n"; + print << "EOM"; +$vname has style problems, please review. + +If any of these errors are false positives, please report +them to the maintainer, see CHECKPATCH in MAINTAINERS. +EOM } return $clean; From 0adb2f1d0b8e8cd38a550c4f130c3a10fa9d9a68 Mon Sep 17 00:00:00 2001 From: Matt Sealey Date: Thu, 19 Apr 2012 16:54:34 -0500 Subject: [PATCH 18/22] siihdmi: strncmp would nuke if the mode in the current monitor database had no name --- drivers/video/mxc/siihdmi.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/video/mxc/siihdmi.c b/drivers/video/mxc/siihdmi.c index 1522c549f16..497bd67552c 100644 --- a/drivers/video/mxc/siihdmi.c +++ b/drivers/video/mxc/siihdmi.c @@ -694,8 +694,9 @@ static int siihdmi_find_vic_from_modedb(const struct fb_videomode *mode) * watch out porting this as it relies the CEA VIC nn string * to be in the modedb */ - if (!strncmp(cea_modes[vic].name, mode->name, 10)) - return vic; + if (cea_modes[vic].name && mode->name) + if (!strncmp(cea_modes[vic].name, mode->name, 10)) + return vic; } return 0; } From 73cae9887ade90f45096112357e1213307d566d3 Mon Sep 17 00:00:00 2001 From: Matt Sealey Date: Thu, 19 Apr 2012 17:46:57 -0500 Subject: [PATCH 19/22] siihdmi: enable DEBUG so that DBG() messages are output --- drivers/video/mxc/siihdmi.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/video/mxc/siihdmi.c b/drivers/video/mxc/siihdmi.c index 497bd67552c..430500a3ba1 100644 --- a/drivers/video/mxc/siihdmi.c +++ b/drivers/video/mxc/siihdmi.c @@ -29,6 +29,7 @@ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ +#define DEBUG #include #include #include From 455affb66eeea08470e11a05a69ff0f4e8158106 Mon Sep 17 00:00:00 2001 From: Matt Sealey Date: Thu, 19 Apr 2012 19:15:49 -0500 Subject: [PATCH 20/22] siihdmi: reintroduce the 480-600ms sleep after reset to allow hotplug to settle. --- drivers/video/mxc/siihdmi.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/video/mxc/siihdmi.c b/drivers/video/mxc/siihdmi.c index 430500a3ba1..2415383bbca 100644 --- a/drivers/video/mxc/siihdmi.c +++ b/drivers/video/mxc/siihdmi.c @@ -205,8 +205,10 @@ static int siihdmi_initialise(struct siihdmi_tx *tx) int ret; /* step 1: reset and initialise */ - if (tx->platform->reset) + if (tx->platform->reset) { tx->platform->reset(); + msleep(480); + } ret = i2c_smbus_write_byte_data(tx->client, SIIHDMI_TPI_REG_RQB, 0x00); if (ret < 0) { From 6be1fd20cc0fb1fc4a794afdbc2125503a398c8d Mon Sep 17 00:00:00 2001 From: Matt Sealey Date: Fri, 20 Apr 2012 11:15:22 -0500 Subject: [PATCH 21/22] efikamx: correct IPU revision again (it should be 2 for IPUv3EX however unintuitive that is, and 3 for IPUv3M on MX53). This fixes the IPU driver getting the correct base address. --- arch/arm/mach-mx5/mx51_efikamx_display.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/mach-mx5/mx51_efikamx_display.c b/arch/arm/mach-mx5/mx51_efikamx_display.c index 2407b75e92c..2ef3453573f 100644 --- a/arch/arm/mach-mx5/mx51_efikamx_display.c +++ b/arch/arm/mach-mx5/mx51_efikamx_display.c @@ -44,7 +44,7 @@ extern void mx5_ipu_reset(void); static struct mxc_ipu_config mxc_ipu_data = { - .rev = 3, + .rev = 2, /* IPUv3EX (MX51) is 2, IPUv3M (MX53) is 3 */ .reset = mx5_ipu_reset, }; From 44a6231768bc06479d264a893dedb9f0c179b84f Mon Sep 17 00:00:00 2001 From: Matt Sealey Date: Fri, 20 Apr 2012 12:37:20 -0500 Subject: [PATCH 22/22] siihdmi: introduce siihdmi.nohotplug=1 argument to disable the hotplug handler globally --- drivers/video/mxc/siihdmi.c | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/drivers/video/mxc/siihdmi.c b/drivers/video/mxc/siihdmi.c index 2415383bbca..3b9dd6686e5 100644 --- a/drivers/video/mxc/siihdmi.c +++ b/drivers/video/mxc/siihdmi.c @@ -60,20 +60,20 @@ /* module parameters */ static unsigned int bus_timeout = 50; module_param(bus_timeout, uint, 0644); -MODULE_PARM_DESC(bus_timeout, "bus timeout in milliseconds"); +MODULE_PARM_DESC(bus_timeout, "Bus timeout in milliseconds"); static unsigned int seventwenty = 1; module_param(seventwenty, uint, 0644); -MODULE_PARM_DESC(seventwenty, "attempt to use 720p mode"); +MODULE_PARM_DESC(seventwenty, "Attempt to use 720p mode"); static unsigned int teneighty; module_param(teneighty, uint, 0644); -MODULE_PARM_DESC(teneighty, "attempt to use 1080p mode"); +MODULE_PARM_DESC(teneighty, "Attempt to use 1080p mode"); static unsigned int useitmodes = 1; module_param(useitmodes, uint, 0644); MODULE_PARM_DESC(useitmodes, - "prefer IT modes over CEA modes when sanitizing the modelist"); + "Prefer IT modes over CEA modes when sanitizing the modelist"); static unsigned int modevic; module_param_named(vic, modevic, uint, 0644); @@ -86,7 +86,11 @@ MODULE_PARM_DESC(forcedvi, "Force DVI sink mode"); static unsigned int useavmute; module_param(useavmute, uint, 0644); -MODULE_PARM_DESC(useavmute, "perform HDMI AV Mute when blanking screen"); +MODULE_PARM_DESC(useavmute, "Perform HDMI AV Mute when blanking screen"); + +static unsigned int nohotplug; +module_param(nohotplug, uint, 0644); +MODULE_PARM_DESC(nohotplug, "Do not install the cable hotplug handler, disabling detection of monitor changes"); static int siihdmi_read_internal(struct siihdmi_tx *tx, u8 page, u8 offset) { @@ -1444,17 +1448,20 @@ static int __devinit siihdmi_probe(struct i2c_client *client, i2c_set_clientdata(client, tx); - INIT_WORK(&tx->hotplug.handler, siihdmi_hotplug_event); + if (!nohotplug) { - BUG_ON(~tx->platform->hotplug.flags & IORESOURCE_IRQ); + INIT_WORK(&tx->hotplug.handler, siihdmi_hotplug_event); - ret = request_irq(tx->platform->hotplug.start, siihdmi_hotplug_handler, + BUG_ON(~tx->platform->hotplug.flags & IORESOURCE_IRQ); + + ret = request_irq(tx->platform->hotplug.start, siihdmi_hotplug_handler, tx->platform->hotplug.flags & IRQF_TRIGGER_MASK, tx->platform->hotplug.name, tx); - if (ret < 0) - WARNING("failed to setup hotplug interrupt: %d\n", ret); - else - tx->hotplug.enabled = true; + if (ret < 0) + WARNING("failed to setup hotplug interrupt: %d\n", ret); + else + tx->hotplug.enabled = true; + } /* initialise the device */ ret = siihdmi_initialise(tx);