[PATCH] Libertas: Fix regression in cmd.c introduced in commit 18c96c3497aa871608d57ca5e08de3558159a6c9

[PATCH] Libertas: Fix regression in cmd.c introduced in commit 18c96c3497aa871608d57ca5e08de3558159a6c9

[Guillaume LECERF]

Hi all,

While reading the last libertas commits, I discovered that this commit
introduced a regression in the libertas wireless driver, inside cmd.c
:

commit 18c96c3497aa871608d57ca5e08de3558159a6c9
[PATCH] libertas: fix WPA associations by handling ENABLE_RSN correctly

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=18c96c3497aa871608d57ca5e08de3558159a6c9

The logic behind has been totally broken, and I think this patch is
worth reviewing.

Regards,

--- a/drivers/net/wireless/libertas/cmd.c
+++ b/drivers/net/wireless/libertas/cmd.c
@@ -241,7 +241,7 @@
                if (*enable)
                        penableRSN->enable = cpu_to_le16(cmd_enable_rsn);
                else
-                       penableRSN->enable = cpu_to_le16(cmd_enable_rsn);
+                       penableRSN->enable = cpu_to_le16(cmd_disable_rsn);
        }

        lbs_deb_leave(LBS_DEB_CMD);

-- 
Guillaume LECERF
GeeXboX developer

Re: [PATCH] Libertas: Fix regression in cmd.c introduced in commit 18c96c3497aa871608d57ca5e08de3558159a6c9

[Satyam Sharma]

Hi Guillaume,

[ Added relevant folks to Cc: ]

On 7/6/07, Guillaume LECERF <glecerf@gmail.com> wrote:
> [...]
>
> --- a/drivers/net/wireless/libertas/cmd.c
> +++ b/drivers/net/wireless/libertas/cmd.c
> @@ -241,7 +241,7 @@
>                 if (*enable)
>                         penableRSN->enable = cpu_to_le16(cmd_enable_rsn);
>                 else
> -                       penableRSN->enable = cpu_to_le16(cmd_enable_rsn);
> +                       penableRSN->enable = cpu_to_le16(cmd_disable_rsn);
>         }
>
>         lbs_deb_leave(LBS_DEB_CMD);
>

Thanks for the patch (looks reasonable to me), but:

1. No point sending a patch without a "Signed-off-by:" line.
2. Changelog (a few lines of explanation before signed-off-by) must
specify what was wrong, and why the patch is correct.
3. Always Cc: relevant maintainers or at least _somebody_ who
_will_ pick it up.
4. Gmail has done horrible things to the patch as usual, use some
other mailer or script.

And in general, please read: Documentation/SubmittingPatches
and http://www.zip.com.au/~akpm/linux/patches/stuff/tpp.txt

Satyam

Re: [PATCH] Libertas: Fix regression in cmd.c introduced in commit 18c96c3497aa871608d57ca5e08de3558159a6c9

[Dan Williams]

On Fri, 2007-07-06 at 12:59 +0530, Satyam Sharma wrote:
> Hi Guillaume,
> 
> [ Added relevant folks to Cc: ]

Botched backport, head is correct.  It's worth noting that WPA didn't
work before the original commit at _all_, and after the commit it would
actually work, but you probably couldn't turn it off :)

Thanks for the catch, I'll fix and push to linville.

dan

> On 7/6/07, Guillaume LECERF <glecerf@gmail.com> wrote:
> > [...]
> >
> > --- a/drivers/net/wireless/libertas/cmd.c
> > +++ b/drivers/net/wireless/libertas/cmd.c
> > @@ -241,7 +241,7 @@
> >                 if (*enable)
> >                         penableRSN->enable = cpu_to_le16(cmd_enable_rsn);
> >                 else
> > -                       penableRSN->enable = cpu_to_le16(cmd_enable_rsn);
> > +                       penableRSN->enable = cpu_to_le16(cmd_disable_rsn);
> >         }
> >
> >         lbs_deb_leave(LBS_DEB_CMD);
> >
> 
> Thanks for the patch (looks reasonable to me), but:
> 
> 1. No point sending a patch without a "Signed-off-by:" line.
> 2. Changelog (a few lines of explanation before signed-off-by) must
> specify what was wrong, and why the patch is correct.
> 3. Always Cc: relevant maintainers or at least _somebody_ who
> _will_ pick it up.
> 4. Gmail has done horrible things to the patch as usual, use some
> other mailer or script.
> 
> And in general, please read: Documentation/SubmittingPatches
> and http://www.zip.com.au/~akpm/linux/patches/stuff/tpp.txt
> 
> Satyam