[Prism54-devel] handle mgmt timeouts by resetting hardware

[Denis Vlasenko]

On Friday 06 August 2004 09:46, Margit Schubert-While wrote:
> More comments :
> Dennis, are you trying to win the "Obfuscated C" prize ?

No. Which part of code made you think so?

> Have you got an aversion to white space ? :
> eg.
>     if (data==(unsigned long)NULL)
>     if(modulation==DOT11_MOD_PBCC) return -ENOTSUPP;

You took those two lines from two different .c files.

First one is from islpci_dev.c, which is a part
of prism54 driver and I use code style consistent
with the rest of the driver.

Second one is from setrate.c. This is new file,
and I think it shouldn't be a part of the driver in the
long run. I will talk with Jean about moving it into wireless.c
as soon as it is used by more than one driver.
At that time, I will change code style to whatever style
Jean will want. Today, I wrote it according
to my style. There is no space before '(' because
in my code I do it that way.

> Re: timer code.
> You seem to have not taken into account PSM
> (Power Saving Mode) and have ignored suspend/resume
> totally.

That's because I did not think about it at all.
My primary testing method is heavy traffic in both directions,
power saving does not kick in :)

I need to look into it. But there is not bug _yet_ because
watchdog timer does not touch hardware _yet_.

> Re: set rates.
> prism54_set_rates :
> Memory leak.
> prism54_set_rates can return early without freeing memory from
> mgt_get_request.
> (Actually, the same applies to the current prism54_set_rate, which
>   just shows that one should not blindly copy code - fix coming up)

Good catch!

Talking about "Obfuscated C" contest:

prism54_set_rate(struct net_device *ndev,
                 struct iw_request_info *info,
                 struct iw_param *vwrq, char *extra)
...
        if ((ret =
             mgt_get_request(priv, DOT11_OID_SUPPORTEDRATES, 0, NULL, &r)))
                return ret;

Please untangle r = f(); and if(r) ...;
--
vda