Stefan Schmidt wrote on 2010-04-13:
> Hello.
>
> On Tue, 2010-04-13 at 09:20, Hennerich, Michael wrote:
>> Stefan Schmidt wrote on 2010-04-12:
>>>> +
>>>> +/*
>>>> + * DEBUG LEVEL
>>>> + * 0 OFF
>>>> + * 1 INFO
>>>> + * 2 INFO + TRACE
>>>> + */
>>>> +
>>>> +#define ADF_DEBUG 0
>>>> +#define DBG(n, args...) do { if (ADF_DEBUG >= (n))
>>>> +pr_debug(args); }
>>> while (0)
>>>
>>> Why do you need an extra layer over the standard debug macros?
>>
>> It allows me to set different types of verbosity.
>> In addition and depending on the Debug Level - these statements get
> completely optimized away by gcc.
>
> I still wonder why you need all this debugging is needed when the
> driver is working and ready for mainline submission.
The driver is ready as far it can be. However I'm not sure you noticed - the
entire mac802154 softmac is still in some work in progress state.
And therefore not yet fully ready for 'Mainline' inclusion.
A few examples:
1) Try to assoc your wpan-phy with a coordinator running on the same phy.
You will notice that it doesn't work.
(which is probably the root cause of the problem you have reported in one of
your earlier mailing list posts).
2) Missing return value handling of xmit().
There should be at least something like this:
RETVAL:
XMIT_SUCCESS
XMIT_SUCCESS_DATPEND
XMIT_FAILURE_CSMACA
XMIT_FAILURE_NOACK
And upper layer should respond with appropriate actions.
Since stuff is still in flux - I really prefer to keep debug options in the
driver for now.
>
>>> Wrong indentation.
>>
>> Well - looking at the source I made this patch from, the indention
>> looks right. This applies to all your highlighted indention problems. I
>> think the problem is caused by my outlook email client. Shame on me :-)
>
> The you should consider fixing this setup: Documentation/email-
> clients.txt
There is not short term option for me right now.
I'll need to submit a patch as attachment.
Unfortunately this mailing list has a message size limit of 40kB.
So inline patch and attachment got rejected.
>
>>>> +static int adf7242_read_fbuf(struct adf7242_local *lp,
>>>> + u8 *data, u8 *len, u8 *lqi) {
>>>> + u8 *buf = lp->buf;
>>>> + int status;
>>>> + struct spi_message msg;
>>>> + struct spi_transfer xfer_head = {
>>>> + .len = 3,
>>>> + .tx_buf = buf,
>>>> + .rx_buf = buf,
>>>> +
>>>> + };
>>>> + struct spi_transfer xfer_buf = {
>>>> + .len = *len,
>>>> + .rx_buf = data,
>>>> + };
>>>> +
>>>> + DBG(2, "%s :Enter\n", __func__);
>>>> + adf7242_wait_ready(lp);
>>>> +
>>>> + mutex_lock(&lp->bmux);
>>>
>>> Move this mutex around spi_sync as in the other functions?
>>
>> This mutex is supposed to also protect lp->buf.
>
> Hmm, I don't see the need for this. Care to elaborate?
Same lp->buf storage is used in read_fbuf() and write_fbuf().
Actually there is no good reason, why buf couldn't be on the stack.
I'll fix it.
Greetings,
Michael
Analog Devices GmbH Wilhelm-Wagenfeld-Str. 6 80807 Muenchen
Sitz der Gesellschaft Muenchen, Registergericht Muenchen HRB 4036
Geschaeftsfuehrer Thomas Wessel, William A. Martin, Margaret Seif
------------------------------------------------------------------------------
Download Intel® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
_______________________________________________
Linux-zigbee-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-zigbee-devel
