Re: [PATCH] staging: wilc1000: Set all options in region debugfs file
On Tue, Aug 18, 2015 at 08:01:00PM -0700, Greg KH wrote: On Tue, Aug 18, 2015 at 10:32:17PM +0530, Chandra S Gorentla wrote: This patch allows setting all options in the module's debug region options file 'wilc_debug_region'. This functionality allows the user to enable logging from all regions (initialization, locks, firmware etc.) of the driver. Logging from the following regions is enabled during the driver initialization: INIT_DBG, GENERIC_DBG, CFG80211_DBG, FIRM_DBG and HOSTAPD_DBG Before this change, the numerical value set is equal first byte of input minus 0x30 (ASCII value of '0'). Because of this, after a write to this debugfs file, it is difficult to predict the regions on which logging is enabled. The DBG_REGION_ALL now includes 3 additional regions TCP_ENH, SPIN_DEBUG and FIRM_DBG. Why did you add these extra ones? I added them because there is code support them and to avoid a holes in the range of the options. All of this should eventually just be deleted, as network drivers need to use the networking driver debug interfaces, not their own crazy ones. In that case, can I assume that we are not going forward with this change? thanks, greg k-h Thank you, chandra -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] staging: wilc1000: Set all options in region debugfs file
On Wed, Aug 19, 2015 at 06:00:12PM +0530, Chandra Gorentla wrote: On Tue, Aug 18, 2015 at 08:01:00PM -0700, Greg KH wrote: On Tue, Aug 18, 2015 at 10:32:17PM +0530, Chandra S Gorentla wrote: This patch allows setting all options in the module's debug region options file 'wilc_debug_region'. This functionality allows the user to enable logging from all regions (initialization, locks, firmware etc.) of the driver. Logging from the following regions is enabled during the driver initialization: INIT_DBG, GENERIC_DBG, CFG80211_DBG, FIRM_DBG and HOSTAPD_DBG Before this change, the numerical value set is equal first byte of input minus 0x30 (ASCII value of '0'). Because of this, after a write to this debugfs file, it is difficult to predict the regions on which logging is enabled. The DBG_REGION_ALL now includes 3 additional regions TCP_ENH, SPIN_DEBUG and FIRM_DBG. Why did you add these extra ones? I added them because there is code support them and to avoid a holes in the range of the options. But why do you need to debug such things? All of this should eventually just be deleted, as network drivers need to use the networking driver debug interfaces, not their own crazy ones. In that case, can I assume that we are not going forward with this change? I hope not, please work on fixing the driver up to work properly (i.e. not with this type of stuff...) thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] staging: wilc1000: Set all options in region debugfs file
This patch allows setting all options in the module's debug region options file 'wilc_debug_region'. This functionality allows the user to enable logging from all regions (initialization, locks, firmware etc.) of the driver. Logging from the following regions is enabled during the driver initialization: INIT_DBG, GENERIC_DBG, CFG80211_DBG, FIRM_DBG and HOSTAPD_DBG Before this change, the numerical value set is equal first byte of input minus 0x30 (ASCII value of '0'). Because of this, after a write to this debugfs file, it is difficult to predict the regions on which logging is enabled. The DBG_REGION_ALL now includes 3 additional regions TCP_ENH, SPIN_DEBUG and FIRM_DBG. Before this change, the region flag FIRM_DBG is enabled during initialization but cleared after a write to the debugfs file. Signed-off-by: Chandra S Gorentla csgoren...@gmail.com --- It is verified by code walk that while generating logs for the flags TCP_ENH and SPIN_DEBUG code does not try to print strings that are not terminated by NULL and variables do not try to access invalid memory locations. drivers/staging/wilc1000/wilc_debugfs.c | 24 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/drivers/staging/wilc1000/wilc_debugfs.c b/drivers/staging/wilc1000/wilc_debugfs.c index ae11186..70d98ee 100644 --- a/drivers/staging/wilc1000/wilc_debugfs.c +++ b/drivers/staging/wilc1000/wilc_debugfs.c @@ -24,7 +24,10 @@ static struct dentry *wilc_dir; * */ -#define DBG_REGION_ALL (GENERIC_DBG | HOSTAPD_DBG | HOSTINF_DBG | CORECONFIG_DBG | CFG80211_DBG | INT_DBG | TX_DBG | RX_DBG | LOCK_DBG | INIT_DBG | BUS_DBG | MEM_DBG) +#define DBG_REGION_ALL (GENERIC_DBG | HOSTAPD_DBG | HOSTINF_DBG | \ + CORECONFIG_DBG | CFG80211_DBG | INT_DBG | \ + TX_DBG | RX_DBG | LOCK_DBG | INIT_DBG | \ + BUS_DBG | MEM_DBG | TCP_ENH | SPIN_DEBUG | FIRM_DBG) #define DBG_LEVEL_ALL (DEBUG | INFO | WRN | ERR) atomic_t REGION = ATOMIC_INIT(INIT_DBG | GENERIC_DBG | CFG80211_DBG | FIRM_DBG | HOSTAPD_DBG); atomic_t DEBUG_LEVEL = ATOMIC_INIT(ERR); @@ -87,23 +90,20 @@ static ssize_t wilc_debug_region_read(struct file *file, char __user *userbuf, s return simple_read_from_buffer(userbuf, count, ppos, buf, res); } -static ssize_t wilc_debug_region_write(struct file *filp, const char *buf, size_t count, loff_t *ppos) +static ssize_t wilc_debug_region_write(struct file *filp, + const char __user *buf, + size_t count, loff_t *ppos) { - char buffer[128] = {}; int flag; + int ret; - if (count sizeof(buffer)) - return -EINVAL; - - if (copy_from_user(buffer, buf, count)) { - return -EFAULT; - } - - flag = buffer[0] - '0'; + ret = kstrtouint_from_user(buf, count, 16, flag); + if (ret) + return ret; if (flag DBG_REGION_ALL) { printk(%s, value (0x%08x) is out of range, stay previous flag (0x%08x)\n, __func__, flag, atomic_read(REGION)); - return -EFAULT; + return -EINVAL; } atomic_set(REGION, (int)flag); -- 2.5.0 -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] staging: wilc1000: Set all options in region debugfs file
On Tue, Aug 18, 2015 at 10:32:17PM +0530, Chandra S Gorentla wrote: This patch allows setting all options in the module's debug region options file 'wilc_debug_region'. This functionality allows the user to enable logging from all regions (initialization, locks, firmware etc.) of the driver. Logging from the following regions is enabled during the driver initialization: INIT_DBG, GENERIC_DBG, CFG80211_DBG, FIRM_DBG and HOSTAPD_DBG Before this change, the numerical value set is equal first byte of input minus 0x30 (ASCII value of '0'). Because of this, after a write to this debugfs file, it is difficult to predict the regions on which logging is enabled. The DBG_REGION_ALL now includes 3 additional regions TCP_ENH, SPIN_DEBUG and FIRM_DBG. Why did you add these extra ones? All of this should eventually just be deleted, as network drivers need to use the networking driver debug interfaces, not their own crazy ones. thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html