Re: [RFC][PATCH 3/8] OMAP: DSS2: Modify dss_recheck_connections
On Thu, 2010-07-01 at 12:31 +0200, ext Archit Taneja wrote: From: Sumit Semwal sumit.sem...@ti.com The addition of the new 2lcd manager requires modifications in the dss_recheck_connections patch, this function behaves the same if the 2lcd manager doesn't exist Here (and also in the previous patch) you talk about 2lcd, but the code uses lcd2. Tomi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC][PATCH 3/8] OMAP: DSS2: Modify dss_recheck_connections
On Thu, 2010-07-01 at 12:31 +0200, ext Archit Taneja wrote: From: Sumit Semwal sumit.sem...@ti.com The addition of the new 2lcd manager requires modifications in the dss_recheck_connections patch, this function behaves the same if the 2lcd manager doesn't exist Here (and also in the previous patch) you talk about 2lcd, but the code uses lcd2. Archit: The new manager's name defined in manager.c is 2lcd, The channel's name in the enum is OMAP_DSS_CHANNEL_LCD2, and the overlay manager enum is OMAP_DSS_OVL_MGR_LCD2. 2lcd as the manager name was taken so that no one makes mistakes while reading/writing sysfs entries. Also, in the dss2 code we use strncmp() to compare the manager names in overlay_manager_store(). Now, if we name the new manager as lcd2, consider the following scenario: Suppose an overlay has its manager presently set as lcd2. Now someone does a sysfs entry to make the manager as lcd. So in this case the variable buf is lcd and and mgr-name is lcd2. The variable len is calculated on the basis of buf (lcd) as 3. So strncmp just compares the first 3 chars, and in that case lcd and lcd2 are the same, luckily this doesn't cause an issue as the first manager fetched by omap_dss_get_overlay_manager() is lcd. If the manager_list was filled in some other way then this would have resulted an issue. In my opinion, len should be calculated as: len = max(len, strlen(mgr-name)); This way strncmp would work correctly in all the cases. In order to take no risks, we named it as 2lcd. Thanks, Archit -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC][PATCH 3/8] OMAP: DSS2: Modify dss_recheck_connections
On Thu, 2010-07-01 at 14:28 +0200, ext Taneja, Archit wrote: On Thu, 2010-07-01 at 12:31 +0200, ext Archit Taneja wrote: From: Sumit Semwal sumit.sem...@ti.com The addition of the new 2lcd manager requires modifications in the dss_recheck_connections patch, this function behaves the same if the 2lcd manager doesn't exist Here (and also in the previous patch) you talk about 2lcd, but the code uses lcd2. Archit: The new manager's name defined in manager.c is 2lcd, The channel's name in the enum is OMAP_DSS_CHANNEL_LCD2, and the overlay manager enum is OMAP_DSS_OVL_MGR_LCD2. 2lcd as the manager name was taken so that no one makes mistakes while reading/writing sysfs entries. Also, in the dss2 code we use strncmp() to compare the manager names in overlay_manager_store(). Now, if we name the new manager as lcd2, consider the following scenario: Suppose an overlay has its manager presently set as lcd2. Now someone does a sysfs entry to make the manager as lcd. So in this case the variable buf is lcd and and mgr-name is lcd2. The variable len is calculated on the basis of buf (lcd) as 3. So strncmp just compares the first 3 chars, and in that case lcd and lcd2 are the same, luckily this doesn't cause an issue as the first manager fetched by omap_dss_get_overlay_manager() is lcd. If the manager_list was filled in some other way then this would have resulted an issue. If the DSS sysfs code if broken, let's rather fix it than name the manager with a rather strange name, just to circumvent a bug. In my opinion, len should be calculated as: len = max(len, strlen(mgr-name)); This way strncmp would work correctly in all the cases. sysfs_streq() could be used there. Tomi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC][PATCH 3/8] OMAP: DSS2: Modify dss_recheck_connections
Hi, On Thu, 2010-07-01 at 14:28 +0200, ext Taneja, Archit wrote: On Thu, 2010-07-01 at 12:31 +0200, ext Archit Taneja wrote: From: Sumit Semwal sumit.sem...@ti.com The addition of the new 2lcd manager requires modifications in the dss_recheck_connections patch, this function behaves the same if the 2lcd manager doesn't exist Here (and also in the previous patch) you talk about 2lcd, but the code uses lcd2. Archit: The new manager's name defined in manager.c is 2lcd, The channel's name in the enum is OMAP_DSS_CHANNEL_LCD2, and the overlay manager enum is OMAP_DSS_OVL_MGR_LCD2. 2lcd as the manager name was taken so that no one makes mistakes while reading/writing sysfs entries. Also, in the dss2 code we use strncmp() to compare the manager names in overlay_manager_store(). Now, if we name the new manager as lcd2, consider the following scenario: Suppose an overlay has its manager presently set as lcd2. Now someone does a sysfs entry to make the manager as lcd. So in this case the variable buf is lcd and and mgr-name is lcd2. The variable len is calculated on the basis of buf (lcd) as 3. So strncmp just compares the first 3 chars, and in that case lcd and lcd2 are the same, luckily this doesn't cause an issue as the first manager fetched by omap_dss_get_overlay_manager() is lcd. If the manager_list was filled in some other way then this would have resulted an issue. If the DSS sysfs code if broken, let's rather fix it than name the manager with a rather strange name, just to circumvent a bug. In my opinion, len should be calculated as: len = max(len, strlen(mgr-name)); This way strncmp would work correctly in all the cases. sysfs_streq() could be used there. I agree, I will make this change. Archit-- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
