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

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

On 02/06/2012 05:05 PM, Vasanthakumar Thiagarajan wrote:
> On Mon, Feb 06, 2012 at 02:37:29PM +0200, Kalle Valo wrote:
>> On 02/06/2012 11:06 AM, Vasanthakumar Thiagarajan wrote:
>>
>>> 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'.
> 
> I think you mean s/user_buf/buf.

Correct.

> Are you not making sure that the
> length of the data is not more than the requested one which is
> passed to copy_to_user() so that read() is always called with
> *ppos=0?. The following code seems to do that

But the function is called multiple times with increasing values of
*ppos as more data is returned to user space:

[  100.303747] ath6kl_fwlog_block_read(): *ppos 0
[  100.305252] ath6kl_fwlog_block_read(): *ppos 30116
[  101.768947] ath6kl_fwlog_block_read(): *ppos 31624
[  117.027469] ath6kl_fwlog_block_read(): *ppos 33124
[  117.090146] ath6kl_fwlog_block_read(): *ppos 34628
[  117.172338] ath6kl_fwlog_block_read(): *ppos 36128

So I can't assume that *ppos = 0.

Kalle