Re: [PATCH 2/2] ath6kl: add blocking debugfs file for retrieving firmware logs

[Kalle Valo <kvalo@qca.qualcomm.com>]

On 02/06/2012 11:06 AM, Vasanthakumar Thiagarajan wrote:
> On Mon, Feb 06, 2012 at 08:23:40AM +0200, Kalle Valo wrote:
>> When debugging firmware issues it's not always enough to get
>> the latest firmware logs, sometimes we need to get logs from a longer
>> period. To make this possible, add a debugfs file named fwlog_block. When
>> reading from this file ath6kl will send firmware logs whenever available
>> and otherwise it will block and wait for new logs.
>>
>> Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>

[...]

>> +	not_copied = copy_to_user(user_buf, buf, len);
>> +	if (not_copied != 0) {
>> +		ret_cnt = -EFAULT;
>> +		goto out;
>> +	}
>> +
>> +	*ppos = *ppos + len;
> 
> Why not to use simple_read_from_buffer()?, looks like it can also
> takes care of len == 0 case in the following check.
> 
> if (pos >= available || !count)
>         return 0;
> 
> when available (len) is 0, pos = available with
> ath6kl_fwlog_block_read().

I actually used simple_read_from_buffer() first, but the problem is that
it assumes that there's just one buffer from which the data is copied.
But in this case there can be multiple buffers from which I copy data.

Ok, that was a bit confusing, let's try to explain a bit differently :)

If 'ppos > 0' (for example during the second function call)
simple_read_from_buffer() will try to copy from 'user_buf + ppos' but I
would want to copy from 'user_buf'.

Kalle