On 2 October 2014 14:53, Rajkumar Manoharan <rmanohar@xxxxxxxxxxxxxxxx> wrote:
> On Thu, Oct 02, 2014 at 02:37:13PM +0200, Michal Kazior wrote:
>> On 2 October 2014 14:12, Rajkumar Manoharan <rmanohar@xxxxxxxxxxxxxxxx> wrote:
>> > Add support to configure packet log filters (tx, rx, rate control)
>> > via debugfs. To disable htt pktlog events set the filters to 0.
>> [...]
>> > +static ssize_t ath10k_write_pktlog_filter(struct file *file,
>> > + const char __user *ubuf,
>> > + size_t count, loff_t *ppos)
>> > +{
>> > + struct ath10k *ar = file->private_data;
>> > + u32 filter;
>> > + int ret;
>> > +
>> > + if (kstrtouint_from_user(ubuf, count, 0, &filter))
>> > + return -EINVAL;
>> > +
>> > + mutex_lock(&ar->conf_mutex);
>> > + if (filter && (filter != ar->debug.pktlog_filter))
>> > + ret = ath10k_wmi_pdev_pktlog_enable(ar, filter);
>> > + else
>> > + ret = ath10k_wmi_pdev_pktlog_disable(ar);
>>
>> You're not checking if ar->state is ON.
>>
> How about updating pktlog_filter alone and not sending WMI command when
> state != ON. So that in the next drv_start pktlog will be either be
> disabled or enabled based on filter value at debug_start.
Sounds good to me.
>> Even if I assume the above doesn't crash the driver/system this has
>> strange semantics. On one hand the driver has to be after drv_start
>> (e.g. at least one interface must be up) before it's possible to set
>> it but on the other hand pktlog_filter is retained across
>> drv_stop/start. I'd expect it to be possible to set the pktlog_filter
>> regardless of driver state but maybe that's just me.
>>
> So pktlog should be disabed at ath10k_debug_stop. isn't it?
Actually my bad. You set ar->debug.pktlog_filter regardless of wmi
command result which is fine.
Michał
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
