lcd_extern: driver defect clean up
authorEvoke Zhang <evoke.zhang@amlogic.com>
Mon, 4 Sep 2017 08:41:42 +0000 (16:41 +0800)
committerJianxin Pan <jianxin.pan@amlogic.com>
Tue, 12 Sep 2017 09:22:21 +0000 (02:22 -0700)
PD#150081: driver defect clean up:
#20
#38
#87
#468
#469
#470
#471
#613
#668

Change-Id: Ifa0756e25088599571761b33efaffb6c2c898dfc
Signed-off-by: Evoke Zhang <evoke.zhang@amlogic.com>
drivers/amlogic/media/vout/lcd/lcd_extern/ext_default.c
drivers/amlogic/media/vout/lcd/lcd_extern/i2c_T5800Q.c
drivers/amlogic/media/vout/lcd/lcd_extern/lcd_extern.c

index 5ef6b50..c8b2ed3 100644 (file)
@@ -181,7 +181,7 @@ static int lcd_extern_power_cmd_dynamic_size(unsigned char *init_table,
 
        switch (ext_config->type) {
        case LCD_EXTERN_I2C:
-               while (i <= max_len) {
+               while ((i + 2) < max_len) {
                        type = init_table[i];
                        if (type == LCD_EXTERN_INIT_END)
                                break;
@@ -191,6 +191,9 @@ static int lcd_extern_power_cmd_dynamic_size(unsigned char *init_table,
                                        init_table[i], init_table[i+1]);
                        }
                        cmd_size = init_table[i+1];
+                       if ((i + 2 + cmd_size) > max_len)
+                               break;
+
                        if (type == LCD_EXTERN_INIT_NONE) {
                                if (cmd_size < 1) {
                                        EXTERR("step %d: invalid cmd_size %d\n",
@@ -238,7 +241,7 @@ static int lcd_extern_power_cmd_dynamic_size(unsigned char *init_table,
                }
                break;
        case LCD_EXTERN_SPI:
-               while (i <= max_len) {
+               while ((i + 2) < max_len) {
                        type = init_table[i];
                        if (type == LCD_EXTERN_INIT_END)
                                break;
@@ -248,6 +251,9 @@ static int lcd_extern_power_cmd_dynamic_size(unsigned char *init_table,
                                        init_table[i], init_table[i+1]);
                        }
                        cmd_size = init_table[i+1];
+                       if ((i + 2 + cmd_size) > max_len)
+                               break;
+
                        if (type == LCD_EXTERN_INIT_NONE) {
                                if (cmd_size < 1) {
                                        EXTERR("step %d: invalid cmd_size %d\n",
@@ -311,7 +317,7 @@ static int lcd_extern_power_cmd_fixed_size(unsigned char *init_table, int flag)
        cmd_size = ext_config->cmd_size;
        switch (ext_config->type) {
        case LCD_EXTERN_I2C:
-               while (i <= max_len) {
+               while ((i + cmd_size) <= max_len) {
                        type = init_table[i];
                        if (type == LCD_EXTERN_INIT_END)
                                break;
@@ -346,7 +352,7 @@ static int lcd_extern_power_cmd_fixed_size(unsigned char *init_table, int flag)
                }
                break;
        case LCD_EXTERN_SPI:
-               while (i <= max_len) {
+               while ((i + cmd_size) <= max_len) {
                        type = init_table[i];
                        if (type == LCD_EXTERN_INIT_END)
                                break;
@@ -494,10 +500,12 @@ static int aml_default_i2c_probe(struct i2c_client *client,
                const struct i2c_device_id *id)
 {
 
-       if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
+       if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
                EXTERR("%s: functionality check failed\n", __func__);
-       else
-               aml_default_i2c_client = client;
+               return -ENODEV;
+       }
+
+       aml_default_i2c_client = client;
 
        EXTPR("%s OK\n", __func__);
        return 0;
@@ -546,6 +554,7 @@ int aml_lcd_extern_default_probe(struct aml_lcd_extern_driver_s *ext_drv)
                }
 
                strncpy(i2c_info.type, ext_drv->config.name, I2C_NAME_SIZE);
+               i2c_info.type[I2C_NAME_SIZE-1] = '\0';
                i2c_info.addr = ext_drv->config.i2c_addr;
                i2c_info.platform_data = &ext_drv->config;
                i2c_info.flags = 0;
@@ -558,11 +567,11 @@ int aml_lcd_extern_default_probe(struct aml_lcd_extern_driver_s *ext_drv)
                if (!i2c_client) {
                        EXTERR("%s failed to new i2c device\n",
                                ext_drv->config.name);
-               } else {
-                       if (lcd_debug_print_flag) {
-                               EXTPR("%s new i2c device succeed\n",
-                                       ext_drv->config.name);
-                       }
+                       return -1;
+               }
+               if (lcd_debug_print_flag) {
+                       EXTPR("%s new i2c device succeed\n",
+                               ext_drv->config.name);
                }
 
                if (!aml_default_i2c_client) {
index b8f6648..94df58a 100644 (file)
@@ -136,7 +136,7 @@ static int lcd_extern_power_cmd(unsigned char *init_table, int flag)
        else
                max_len = LCD_EXTERN_INIT_OFF_MAX;
 
-       while (i <= max_len) {
+       while ((i + cmd_size) <= max_len) {
                if (init_table[i] == LCD_EXTERN_INIT_END)
                        break;
                if (lcd_debug_print_flag) {
@@ -216,7 +216,10 @@ static int lcd_extern_driver_update(struct aml_lcd_extern_driver_s *ext_drv)
        return 0;
 }
 
-/* debug function */
+/* *********************************************************
+ * debug function
+ * *********************************************************
+ */
 static const char *lcd_extern_debug_usage_str = {
 "Usage:\n"
 "    echo <reg> <> ... <> > write ; T5800Q i2c command write, 7 parameters without address\n"
@@ -301,10 +304,12 @@ static int aml_T5800Q_i2c_probe(struct i2c_client *client,
                const struct i2c_device_id *id)
 {
 
-       if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
+       if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
                EXTERR("%s: functionality check failed\n", __func__);
-       else
-               aml_T5800Q_i2c_client = client;
+               return -ENODEV;
+       }
+
+       aml_T5800Q_i2c_client = client;
 
        EXTPR("%s OK\n", __func__);
        return 0;
@@ -348,6 +353,7 @@ int aml_lcd_extern_i2c_T5800Q_probe(struct aml_lcd_extern_driver_s *ext_drv)
        }
 
        strncpy(i2c_info.type, ext_drv->config.name, I2C_NAME_SIZE);
+       i2c_info.type[I2C_NAME_SIZE-1] = '\0';
        i2c_info.addr = ext_drv->config.i2c_addr;
        i2c_info.platform_data = &ext_drv->config;
        i2c_info.flags = 0;
@@ -359,12 +365,10 @@ int aml_lcd_extern_i2c_T5800Q_probe(struct aml_lcd_extern_driver_s *ext_drv)
        i2c_client = i2c_new_device(adapter, &i2c_info);
        if (!i2c_client) {
                EXTERR("%s failed to new i2c device\n", ext_drv->config.name);
-       } else {
-               if (lcd_debug_print_flag) {
-                       EXTPR("%s new i2c device succeed\n",
-                               ext_drv->config.name);
-               }
+               return -1;
        }
+       if (lcd_debug_print_flag)
+               EXTPR("%s new i2c device succeed\n", ext_drv->config.name);
 
        if (!aml_T5800Q_i2c_client) {
                ret = i2c_add_driver(&aml_T5800Q_i2c_driver);
index 2490664..f768c7c 100644 (file)
@@ -249,7 +249,7 @@ static int lcd_extern_init_table_dynamic_size_load_dts(
        }
 
        i = 0;
-       while (i < max_len) {
+       while ((i + 2) < max_len) { /* type & cmd_size detect */
                /* step1: type */
                ret = of_property_read_u32_index(of_node, propname, i, &val);
                if (ret) {
@@ -277,6 +277,8 @@ static int lcd_extern_init_table_dynamic_size_load_dts(
                        i += 2;
                        continue;
                }
+               if ((i + 2 + cmd_size) > max_len)
+                       break;
                /* step3: data */
                for (j = 0; j < cmd_size; j++) {
                        ret = of_property_read_u32_index(
@@ -323,7 +325,7 @@ static int lcd_extern_init_table_fixed_size_load_dts(
        }
 
        i = 0;
-       while (i < max_len) {
+       while ((i + cmd_size) <= max_len) { /* group detect */
                for (j = 0; j < cmd_size; j++) {
                        ret = of_property_read_u32_index(
                                of_node, propname, (i+j), &val);
@@ -362,29 +364,32 @@ static int lcd_extern_get_config_dts(struct device_node *of_node,
        extconf->table_init_loaded = 0;
 
        ret = of_property_read_u32(of_node, "index", &val);
-       if (ret)
+       if (ret) {
                EXTERR("get index failed, exit\n");
-       else
-               extconf->index = (unsigned char)val;
+               return -1;
+       }
+       extconf->index = (unsigned char)val;
 
        ret = of_property_read_string(of_node, "status", &str);
        if (ret) {
                EXTERR("get index %d status failed\n", extconf->index);
-       } else {
-               if (lcd_debug_print_flag)
-                       EXTPR("index %d status = %s\n", extconf->index, str);
-               if (strncmp(str, "okay", 2) == 0)
-                       extconf->status = 1;
-               else
-                       return -1;
+               return -1;
        }
+       if (lcd_debug_print_flag)
+               EXTPR("index %d status = %s\n", extconf->index, str);
+       if (strncmp(str, "okay", 2) == 0)
+               extconf->status = 1;
+       else
+               return -1;
 
        ret = of_property_read_string(of_node, "extern_name", &str);
        if (ret) {
                str = "none";
                EXTERR("get extern_name failed\n");
        }
-       strcpy(extconf->name, str);
+       strncpy(extconf->name, str, LCD_EXTERN_NAME_LEN_MAX);
+       /* ensure string ending */
+       extconf->name[LCD_EXTERN_NAME_LEN_MAX-1] = '\0';
        EXTPR("load config: %s[%d]\n", extconf->name, extconf->index);
 
        ret = of_property_read_u32(of_node, "type", &extconf->type);
@@ -401,16 +406,16 @@ static int lcd_extern_get_config_dts(struct device_node *of_node,
                        EXTERR("get %s i2c_address failed, exit\n",
                                extconf->name);
                        extconf->i2c_addr = 0xff;
-               } else {
-                       extconf->i2c_addr = (unsigned char)val;
+                       return -1;
                }
+               extconf->i2c_addr = (unsigned char)val;
                if (lcd_debug_print_flag) {
                        EXTPR("%s i2c_address=0x%02x\n",
                                extconf->name, extconf->i2c_addr);
                }
                ret = of_property_read_u32(of_node, "i2c_second_address", &val);
                if (ret) {
-                       EXTERR("get %s i2c_second_address failed, exit\n",
+                       EXTPR("get %s i2c_second_address failed\n",
                                extconf->name);
                        extconf->i2c_addr2 = 0xff;
                } else {
@@ -425,9 +430,9 @@ static int lcd_extern_get_config_dts(struct device_node *of_node,
                if (ret) {
                        EXTERR("get %s i2c_bus failed, exit\n", extconf->name);
                        extconf->i2c_bus = AML_I2C_MASTER_A;
-               } else {
-                       extconf->i2c_bus = lcd_extern_get_i2c_bus_str(str);
+                       return -1;
                }
+               extconf->i2c_bus = lcd_extern_get_i2c_bus_str(str);
                if (lcd_debug_print_flag) {
                        EXTPR("%s i2c_bus=%s[%d]\n",
                                extconf->name, str, extconf->i2c_bus);
@@ -435,7 +440,12 @@ static int lcd_extern_get_config_dts(struct device_node *of_node,
 
                ret = of_property_read_u32(of_node, "cmd_size", &val);
                if (ret) {
-                       EXTERR("get %s cmd_size failed\n", extconf->name);
+                       if (extconf->index == 0) {
+                               EXTERR("get %s cmd_size failed\n",
+                                       extconf->name);
+                       } else {
+                               EXTPR("%s no cmd_size\n", extconf->name);
+                       }
                        extconf->cmd_size = 0;
                } else {
                        extconf->cmd_size = (unsigned char)val;
@@ -445,7 +455,10 @@ static int lcd_extern_get_config_dts(struct device_node *of_node,
                                extconf->name, extconf->cmd_size);
                }
                if (extconf->cmd_size <= 1) {
-                       EXTERR("cmd_size %d is invalid\n", extconf->cmd_size);
+                       if (extconf->index == 0) {
+                               EXTERR("cmd_size %d is invalid\n",
+                                       extconf->cmd_size);
+                       }
                        break;
                }
 
@@ -473,40 +486,34 @@ static int lcd_extern_get_config_dts(struct device_node *of_node,
                        EXTERR("get %s gpio_spi_cs failed, exit\n",
                                extconf->name);
                        extconf->spi_gpio_cs = LCD_EXTERN_GPIO_NUM_MAX;
-               } else {
-                       extconf->spi_gpio_cs = val;
-                       lcd_extern_gpio_register(val);
-                       if (lcd_debug_print_flag) {
-                               EXTPR("spi_gpio_cs: %d\n",
-                                       extconf->spi_gpio_cs);
-                       }
+                       return -1;
                }
+               extconf->spi_gpio_cs = val;
+               lcd_extern_gpio_register(val);
+               if (lcd_debug_print_flag)
+                       EXTPR("spi_gpio_cs: %d\n", extconf->spi_gpio_cs);
                ret = of_property_read_u32(of_node, "gpio_spi_clk", &val);
                if (ret) {
                        EXTERR("get %s gpio_spi_clk failed, exit\n",
                                extconf->name);
                        extconf->spi_gpio_clk = LCD_EXTERN_GPIO_NUM_MAX;
-               } else {
-                       extconf->spi_gpio_clk = val;
-                       lcd_extern_gpio_register(val);
-                       if (lcd_debug_print_flag) {
-                               EXTPR("spi_gpio_clk: %d\n",
-                                       extconf->spi_gpio_clk);
-                       }
+                       return -1;
                }
+               extconf->spi_gpio_clk = val;
+               lcd_extern_gpio_register(val);
+               if (lcd_debug_print_flag)
+                       EXTPR("spi_gpio_clk: %d\n", extconf->spi_gpio_clk);
                ret = of_property_read_u32(of_node, "gpio_spi_data", &val);
                if (ret) {
                        EXTERR("get %s gpio_spi_data failed, exit\n",
                                extconf->name);
                        extconf->spi_gpio_data = LCD_EXTERN_GPIO_NUM_MAX;
-               } else {
-                       extconf->spi_gpio_data = val;
-                       lcd_extern_gpio_register(val);
-                       if (lcd_debug_print_flag) {
-                               EXTPR("spi_gpio_data: %d\n",
-                                       extconf->spi_gpio_data);
-                       }
+                       return -1;
                }
+               extconf->spi_gpio_data = val;
+               lcd_extern_gpio_register(val);
+               if (lcd_debug_print_flag)
+                       EXTPR("spi_gpio_data: %d\n", extconf->spi_gpio_data);
                ret = of_property_read_u32(of_node, "spi_clk_freq", &val);
                if (ret) {
                        EXTERR("get %s spi_clk_freq failed, default to %dHz\n",
@@ -533,7 +540,12 @@ static int lcd_extern_get_config_dts(struct device_node *of_node,
                }
                ret = of_property_read_u32(of_node, "cmd_size", &val);
                if (ret) {
-                       EXTERR("get %s cmd_size failed\n", extconf->name);
+                       if (extconf->index == 0) {
+                               EXTERR("get %s cmd_size failed\n",
+                                       extconf->name);
+                       } else {
+                               EXTPR("%s no cmd_size\n", extconf->name);
+                       }
                        extconf->cmd_size = 0;
                } else {
                        extconf->cmd_size = (unsigned char)val;
@@ -543,7 +555,10 @@ static int lcd_extern_get_config_dts(struct device_node *of_node,
                                extconf->name, extconf->cmd_size);
                }
                if (extconf->cmd_size <= 1) {
-                       EXTERR("cmd_size %d is invalid\n", extconf->cmd_size);
+                       if (extconf->index == 0) {
+                               EXTERR("cmd_size %d is invalid\n",
+                                       extconf->cmd_size);
+                       }
                        break;
                }
 
@@ -619,7 +634,7 @@ static int lcd_extern_init_table_dynamic_size_load_unifykey(
        }
 
        i = 0;
-       while (i < max_len) {
+       while ((i + 2) < max_len) { /* type & cmd_size detect */
                /* step1: type */
                len += 1;
                ret = lcd_unifykey_len_check(key_len, len);
@@ -650,6 +665,8 @@ static int lcd_extern_init_table_dynamic_size_load_unifykey(
                        i += 2;
                        continue;
                }
+               if ((i + 2 + cmd_size) > max_len)
+                       break;
 
                /* step3: data */
                len += cmd_size;
@@ -703,7 +720,7 @@ static int lcd_extern_init_table_fixed_size_load_unifykey(
        }
 
        i = 0;
-       while (i < max_len) {
+       while ((i + cmd_size) <= max_len) { /* group detect */
                len += cmd_size;
                ret = lcd_unifykey_len_check(key_len, len);
                if (ret) {
@@ -778,9 +795,10 @@ static int lcd_extern_get_config_unifykey(struct lcd_extern_config_s *extconf)
 
        /* basic: 33byte */
        p = para + LCD_UKEY_HEAD_SIZE;
-       *(p + LCD_UKEY_EXT_NAME - 1) = '\0'; /* ensure string ending */
        str = (const char *)p;
-       strcpy(extconf->name, str);
+       strncpy(extconf->name, str, LCD_EXTERN_NAME_LEN_MAX);
+       /* ensure string ending */
+       extconf->name[LCD_EXTERN_NAME_LEN_MAX-1] = '\0';
        p += LCD_UKEY_EXT_NAME;
        extconf->index = *p;
        p += LCD_UKEY_EXT_INDEX;
@@ -1159,6 +1177,10 @@ static int lcd_extern_add_driver(struct lcd_extern_config_s *extconf)
        return 0;
 }
 
+/* *********************************************************
+ * debug function
+ * *********************************************************
+ */
 #define EXT_LEN_MAX   200
 static void lcd_extern_init_table_dynamic_size_print(
                struct lcd_extern_config_s *econf, int flag)
@@ -1315,7 +1337,7 @@ static ssize_t lcd_extern_debug_help(struct class *class,
 static ssize_t lcd_extern_info_dump(struct class *class,
                struct class_attribute *attr, const char *buf, size_t count)
 {
-       unsigned int ret;
+       unsigned int ret = 0;
        int i, index;
        struct aml_lcd_extern_driver_s *ext_drv;
 
@@ -1323,8 +1345,12 @@ static ssize_t lcd_extern_info_dump(struct class *class,
        switch (buf[0]) {
        case 'i':
                ret = sscanf(buf, "index %d", &index);
-               ext_drv = aml_lcd_extern_get_driver(index);
-               lcd_extern_config_dump(ext_drv);
+               if (ret == 1) {
+                       ext_drv = aml_lcd_extern_get_driver(index);
+                       lcd_extern_config_dump(ext_drv);
+               } else {
+                       EXTERR("invalid parameters\n");
+               }
                break;
        case 'a':
                for (i = 0; i < lcd_ext_driver_num; i++)
@@ -1388,9 +1414,9 @@ static ssize_t lcd_extern_debug_test_store(struct class *class,
 static struct class_attribute lcd_extern_class_attrs[] = {
        __ATTR(info, 0644,
                lcd_extern_debug_help, lcd_extern_info_dump),
-       __ATTR(key_valid,   0644,
+       __ATTR(key_valid,   0444,
                 lcd_extern_debug_key_valid_show, NULL),
-       __ATTR(config_load, 0644,
+       __ATTR(config_load, 0444,
                lcd_extern_debug_config_load_show, NULL),
        __ATTR(test, 0644,
                lcd_extern_debug_test_show, lcd_extern_debug_test_store),