Plan for ath6kl cleanup

Plan for ath6kl cleanup

[Kalle Valo]

We have been thinking about how to get ath6kl out from staging and get
it to a first class citizen under drivers/net/wireless. There's quite a
lot of work to do get ath6kl cleaned up and the prospect of doing all
that through the staging-next tree wasn't that exciting. We would be
sending hundreds of patches and it would take a long time to cleanup the
driver. And the disconnection from the wireless core development also
sounded very daunting (cfg80211 API changes etc.).

So we started to find a faster way to clean up ath6kl and ended up with
a setup where we would have a separate tree in kernel.org just for
ath6kl clean up. Once we think that the driver is ready, we will take
the driver from the cleanup tree and submit as a single patch for
review. As we will submit the driver as a single commit, history doesn't
matter and this makes it possible to use not so perfect patches in our
tree and save a lot of time.

During the transition period we will still fix the important bugs in the
staging driver, and naturally port them to the cleaned up driver. But
once the cleaned up driver is accepted we plan to remove the staging driver.

We already started the cleanup and created ath6kl-clean git tree here:

http://git.kernel.org/?p=linux/kernel/git/kvalo/ath6kl-cleanup.git;a=summary

The tree is based on wireless-testing and I have cherry picked missing
ath6kl patches from staging-next tree. Also the driver is already moved
to drivers/net/wireless/ath/ath6kl directory to make it easy to submit
the driver for review once it's ready.

Everyone is free to send patches and participate in the ath6kl cleanup.
As we don't want to clutter linux-wireless mailing list with hundreds of
useless "staging" patches, please send the patches to kvalo@adurom.com
(have to use my private email due to buggy email servers) and I will
apply them.

We have irc channel #ath6kl on freenode where some of the ath6kl devs
hang out. Feel free to make any questions also there, just give enough
time for people to answer. But naturally linux-wireless mailing list is
the best place to ask questions.

Also the wiki page contains some information and we will start adding more:

http://wireless.kernel.org/en/users/Drivers/ath6kl

Thoughts?

Kalle

Re: Plan for ath6kl cleanup

[Greg KH]

On Fri, May 20, 2011 at 04:13:24PM +0300, Kalle Valo wrote:
> We have been thinking about how to get ath6kl out from staging and get
> it to a first class citizen under drivers/net/wireless. There's quite a
> lot of work to do get ath6kl cleaned up and the prospect of doing all
> that through the staging-next tree wasn't that exciting. We would be
> sending hundreds of patches and it would take a long time to cleanup the
> driver. And the disconnection from the wireless core development also
> sounded very daunting (cfg80211 API changes etc.).

This sounds like, "We don't like the kernel development model, it
requires us to break everything up into small patches and show our work
to everyone."

Hopefully that is not the case here.

> So we started to find a faster way to clean up ath6kl and ended up with
> a setup where we would have a separate tree in kernel.org just for
> ath6kl clean up. Once we think that the driver is ready, we will take
> the driver from the cleanup tree and submit as a single patch for
> review. As we will submit the driver as a single commit, history doesn't
> matter and this makes it possible to use not so perfect patches in our
> tree and save a lot of time.

Yup, that is the case :(

> During the transition period we will still fix the important bugs in the
> staging driver, and naturally port them to the cleaned up driver. But
> once the cleaned up driver is accepted we plan to remove the staging driver.
> 
> We already started the cleanup and created ath6kl-clean git tree here:
> 
> http://git.kernel.org/?p=linux/kernel/git/kvalo/ath6kl-cleanup.git;a=summary
> 
> The tree is based on wireless-testing and I have cherry picked missing
> ath6kl patches from staging-next tree. Also the driver is already moved
> to drivers/net/wireless/ath/ath6kl directory to make it easy to submit
> the driver for review once it's ready.
> 
> Everyone is free to send patches and participate in the ath6kl cleanup.
> As we don't want to clutter linux-wireless mailing list with hundreds of
> useless "staging" patches, please send the patches to kvalo@adurom.com
> (have to use my private email due to buggy email servers) and I will
> apply them.
> 
> We have irc channel #ath6kl on freenode where some of the ath6kl devs
> hang out. Feel free to make any questions also there, just give enough
> time for people to answer. But naturally linux-wireless mailing list is
> the best place to ask questions.
> 
> Also the wiki page contains some information and we will start adding more:
> 
> http://wireless.kernel.org/en/users/Drivers/ath6kl
> 
> Thoughts?

Why don't I just delete the whole driver from the tree right now as you
all don't seem to understand how to develop stuff within the community
and want to do it all external until some magic "it is done" time when
you want us to take it back?

That's not nice at all.

Don't worry about "cluttering" up the mailing list, send them to me, and
to the driver devel mailing list, and I will take them just fine.

Have you somehow been finding that I not accept patches quick enough?
Do you not like the fact that others comment, review, and contribute
patches for this driver?

This sounds like a very big step back for your development effort, I
strongly advise you to not do this at all.

thanks,

greg k-h

Re: Plan for ath6kl cleanup

[Marcel Holtmann]

Hi Kalle,

> We have been thinking about how to get ath6kl out from staging and get
> it to a first class citizen under drivers/net/wireless. There's quite a
> lot of work to do get ath6kl cleaned up and the prospect of doing all
> that through the staging-next tree wasn't that exciting. We would be
> sending hundreds of patches and it would take a long time to cleanup the
> driver. And the disconnection from the wireless core development also
> sounded very daunting (cfg80211 API changes etc.).

my main question is why do you still bother with this driver and not
re-write it as a mac80211 driver from scratch. It seems to be more
softmac than fullmac anyway. Or am I mistaken here?

The source code of the current staging driver is 5 MB in size. That is
by far the largest staging driver. Even brcm80211 is smaller.

Regards

Marcel

Re: Plan for ath6kl cleanup

[Christoph Hellwig]

On Fri, May 20, 2011 at 06:36:00AM -0700, Greg KH wrote:
> On Fri, May 20, 2011 at 04:13:24PM +0300, Kalle Valo wrote:
> > We have been thinking about how to get ath6kl out from staging and get
> > it to a first class citizen under drivers/net/wireless. There's quite a
> > lot of work to do get ath6kl cleaned up and the prospect of doing all
> > that through the staging-next tree wasn't that exciting. We would be
> > sending hundreds of patches and it would take a long time to cleanup the
> > driver. And the disconnection from the wireless core development also
> > sounded very daunting (cfg80211 API changes etc.).
> 
> This sounds like, "We don't like the kernel development model, it
> requires us to break everything up into small patches and show our work
> to everyone."

It's not the kernel development mode, it's something that you try to
impose on staging users.  Creating a more less new driver from a pig
pile of junk doesn't make sense minimized patches with micro-review.

It's a lot whacking and pretty big changes for things like switching to
mac80211.  The kernel development model works nicely for maintaining
more or less working software, but not for creating something new.

> 
> Hopefully that is not the case here.
> 
> > So we started to find a faster way to clean up ath6kl and ended up with
> > a setup where we would have a separate tree in kernel.org just for
> > ath6kl clean up. Once we think that the driver is ready, we will take
> > the driver from the cleanup tree and submit as a single patch for
> > review. As we will submit the driver as a single commit, history doesn't
> > matter and this makes it possible to use not so perfect patches in our
> > tree and save a lot of time.
> 
> Yup, that is the case :(
> 
> > During the transition period we will still fix the important bugs in the
> > staging driver, and naturally port them to the cleaned up driver. But
> > once the cleaned up driver is accepted we plan to remove the staging driver.
> > 
> > We already started the cleanup and created ath6kl-clean git tree here:
> > 
> > http://git.kernel.org/?p=linux/kernel/git/kvalo/ath6kl-cleanup.git;a=summary
> > 
> > The tree is based on wireless-testing and I have cherry picked missing
> > ath6kl patches from staging-next tree. Also the driver is already moved
> > to drivers/net/wireless/ath/ath6kl directory to make it easy to submit
> > the driver for review once it's ready.
> > 
> > Everyone is free to send patches and participate in the ath6kl cleanup.
> > As we don't want to clutter linux-wireless mailing list with hundreds of
> > useless "staging" patches, please send the patches to kvalo@adurom.com
> > (have to use my private email due to buggy email servers) and I will
> > apply them.
> > 
> > We have irc channel #ath6kl on freenode where some of the ath6kl devs
> > hang out. Feel free to make any questions also there, just give enough
> > time for people to answer. But naturally linux-wireless mailing list is
> > the best place to ask questions.
> > 
> > Also the wiki page contains some information and we will start adding more:
> > 
> > http://wireless.kernel.org/en/users/Drivers/ath6kl
> > 
> > Thoughts?
> 
> Why don't I just delete the whole driver from the tree right now as you
> all don't seem to understand how to develop stuff within the community
> and want to do it all external until some magic "it is done" time when
> you want us to take it back?
> 
> That's not nice at all.
> 
> Don't worry about "cluttering" up the mailing list, send them to me, and
> to the driver devel mailing list, and I will take them just fine.
> 
> Have you somehow been finding that I not accept patches quick enough?
> Do you not like the fact that others comment, review, and contribute
> patches for this driver?
> 
> This sounds like a very big step back for your development effort, I
> strongly advise you to not do this at all.
> 
> thanks,
> 
> greg k-h
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
---end quoted text---

Re: Plan for ath6kl cleanup

[Kalle Valo]

Hi Greg,

thanks for the quick answer!

Greg KH <gregkh@suse.de> writes:

> On Fri, May 20, 2011 at 04:13:24PM +0300, Kalle Valo wrote:
>> We have been thinking about how to get ath6kl out from staging and get
>> it to a first class citizen under drivers/net/wireless. There's quite a
>> lot of work to do get ath6kl cleaned up and the prospect of doing all
>> that through the staging-next tree wasn't that exciting. We would be
>> sending hundreds of patches and it would take a long time to cleanup the
>> driver. And the disconnection from the wireless core development also
>> sounded very daunting (cfg80211 API changes etc.).
>
> This sounds like, "We don't like the kernel development model, it
> requires us to break everything up into small patches and show our work
> to everyone."
>
> Hopefully that is not the case here.

Trust me, that's not the case. Soon we will submit the driver for review
to linux-wireless list and then everyone are more than welcome to review
everything, down to the every bit in the driver.

And seriously, what good does it take to review some ugly patches about
removing OS abstractions, private user space interfaces, fixing coding
style and removing odd directory structures? Wouldn't it be better, and
easier, to review a smaller, leaner and cleaned up driver? At least I
would prefer that. And that's what we are going to do, once our tiger
team has finished the cleanup.

>> So we started to find a faster way to clean up ath6kl and ended up with
>> a setup where we would have a separate tree in kernel.org just for
>> ath6kl clean up. Once we think that the driver is ready, we will take
>> the driver from the cleanup tree and submit as a single patch for
>> review. As we will submit the driver as a single commit, history doesn't
>> matter and this makes it possible to use not so perfect patches in our
>> tree and save a lot of time.
>
> Yup, that is the case :(

See above.

And if you want to have a reason, it's about the staging area is not
really an easy place for active wireless driver development. Don't get
me wrong, staging is good for drivers which don't have that active
development. But ath6kl won't belong to that group anymore and I would
like to take some shortcuts to save time and focus on the real thing,
which is improving quality of the driver and adding more features.

>> During the transition period we will still fix the important bugs in the
>> staging driver, and naturally port them to the cleaned up driver. But
>> once the cleaned up driver is accepted we plan to remove the staging driver.
>> 
>> We already started the cleanup and created ath6kl-clean git tree here:
>> 
>> http://git.kernel.org/?p=linux/kernel/git/kvalo/ath6kl-cleanup.git;a=summary
>> 
>> The tree is based on wireless-testing and I have cherry picked missing
>> ath6kl patches from staging-next tree. Also the driver is already moved
>> to drivers/net/wireless/ath/ath6kl directory to make it easy to submit
>> the driver for review once it's ready.
>> 
>> Everyone is free to send patches and participate in the ath6kl cleanup.
>> As we don't want to clutter linux-wireless mailing list with hundreds of
>> useless "staging" patches, please send the patches to kvalo@adurom.com
>> (have to use my private email due to buggy email servers) and I will
>> apply them.
>> 
>> We have irc channel #ath6kl on freenode where some of the ath6kl devs
>> hang out. Feel free to make any questions also there, just give enough
>> time for people to answer. But naturally linux-wireless mailing list is
>> the best place to ask questions.
>> 
>> Also the wiki page contains some information and we will start adding more:
>> 
>> http://wireless.kernel.org/en/users/Drivers/ath6kl
>> 
>> Thoughts?
>
> Why don't I just delete the whole driver from the tree right now as you
> all don't seem to understand how to develop stuff within the community
> and want to do it all external until some magic "it is done" time when
> you want us to take it back?

Because there are currently users using the driver from staging. That
would be a disservice for them. We help our users better if we keep the
staging driver around and alive until the cleaned up version has been
merged.

> That's not nice at all.

For who? I think for users and for the community it's better that they
get a proper, and properly supported, upstream driver as soon as
possible. The timeframe is something like having a cleaned up version in
few weeks. But working with staging it would take months to get to the
same state.

> Don't worry about "cluttering" up the mailing list, send them to me, and
> to the driver devel mailing list, and I will take them just fine.

I'm talking about linux-wireless mailing list. And yes, I really think
that sending patches like removing A_MEMSET() is cluttering the mailing
list. I would rather see wireless hackers doing something productive
than reading patches like that.

And if someone really likes reading patches like that (I don't) then
they are all available in ath6k-cleanup git tree.

> Have you somehow been finding that I not accept patches quick enough?

Yes, you have been accepting the patches quickly, that's not the
problem. But when we talk about cleaning up the driver properly the flow
of patches increases significantly, at least hundreds of patches from
different people working concurrently. So we would have a huge ping-pong
of patches which conflict each other, you don't like or someone else
doesn't like and has to be submitted again. Managing all that will take
a lot of time and I would rather use that time better improving the
driver than just (re)submitting cleanup patches.

So to be more clear: I expect that if the cleanup would happen in
staging-next tree it would take months to finish but doing it in a
separate tree we can do it in weeks. I'm guessing that we will have a
first version of the driver ready for review in three weeks or so.

> Do you not like the fact that others comment, review, and contribute
> patches for this driver?

Oh yes, I like all that very much, no question about that. I like it so
much that I'm taking extra effort to get the driver to wireless-testing
as soon as possible and start getting the support from the Linux
wireless community. Because, let's face it, wireless community doesn't
care about staging drivers.

Staging is great for some type of drivers, but a very difficult place
for a wireless driver under active development. For example, cfg80211
API changes would be difficult to handle because wireless-testing would
have too old ath6kl and staging-next would have too old wireless core.
Just think all the pain we (Atheros developers) would have to suffer. I
really don't want to waste any time trying to get things working between
the two trees.

> This sounds like a very big step back for your development effort, I
> strongly advise you to not do this at all.

I understand why you are worried. But we have our "top men" working on
this and I'm sure the end result will be good :)

Did my explonation above decrease your worries at all?

Kalle

Re: Plan for ath6kl cleanup

[Kalle Valo]

Marcel Holtmann <marcel@holtmann.org> writes:

> Hi Kalle,

Hi Marcel,

>> We have been thinking about how to get ath6kl out from staging and get
>> it to a first class citizen under drivers/net/wireless. There's quite a
>> lot of work to do get ath6kl cleaned up and the prospect of doing all
>> that through the staging-next tree wasn't that exciting. We would be
>> sending hundreds of patches and it would take a long time to cleanup the
>> driver. And the disconnection from the wireless core development also
>> sounded very daunting (cfg80211 API changes etc.).
>
> my main question is why do you still bother with this driver and not
> re-write it as a mac80211 driver from scratch. It seems to be more
> softmac than fullmac anyway. Or am I mistaken here?

Actually ar6003, the chip ath6kl supports, is a fullmac. It can take
ethernet frames, all authentication/association frames are created by
the firmware and firmware manages the roaming as well. So there isn't
anything mac80211 could do.

> The source code of the current staging driver is 5 MB in size. That is
> by far the largest staging driver. Even brcm80211 is smaller.

Yeah, the driver is huge. But once the cleanup is done it will be a lot
smaller.

Kalle

Re: Plan for ath6kl cleanup

[Joe Perches]

On Fri, 2011-05-20 at 10:29 -0400, Christoph Hellwig wrote:
> On Fri, May 20, 2011 at 06:36:00AM -0700, Greg KH wrote:
> > On Fri, May 20, 2011 at 04:13:24PM +0300, Kalle Valo wrote:
> > > We have been thinking about how to get ath6kl out from staging and get
> > > it to a first class citizen under drivers/net/wireless. There's quite a
> > > lot of work to do get ath6kl cleaned up and the prospect of doing all
> > > that through the staging-next tree wasn't that exciting. We would be
> > > sending hundreds of patches and it would take a long time to cleanup the
> > > driver. And the disconnection from the wireless core development also
> > > sounded very daunting (cfg80211 API changes etc.).
> > 
> > This sounds like, "We don't like the kernel development model, it
> > requires us to break everything up into small patches and show our work
> > to everyone."
> 
> It's not the kernel development mode, it's something that you try to
> impose on staging users.  Creating a more less new driver from a pig
> pile of junk doesn't make sense minimized patches with micro-review.

I tried it once on ath6kl.
Ended up with ~300 patches and a very clean tree.

http://repo.or.cz/w/linux-2.6/trivial-mods.git/shortlog/refs/heads/20110108_ath6kl

The coordination with atheros and greg was balky.

Re: Plan for ath6kl cleanup

[Greg KH]

On Fri, May 20, 2011 at 09:51:21AM -0700, Joe Perches wrote:
> On Fri, 2011-05-20 at 10:29 -0400, Christoph Hellwig wrote:
> > On Fri, May 20, 2011 at 06:36:00AM -0700, Greg KH wrote:
> > > On Fri, May 20, 2011 at 04:13:24PM +0300, Kalle Valo wrote:
> > > > We have been thinking about how to get ath6kl out from staging and get
> > > > it to a first class citizen under drivers/net/wireless. There's quite a
> > > > lot of work to do get ath6kl cleaned up and the prospect of doing all
> > > > that through the staging-next tree wasn't that exciting. We would be
> > > > sending hundreds of patches and it would take a long time to cleanup the
> > > > driver. And the disconnection from the wireless core development also
> > > > sounded very daunting (cfg80211 API changes etc.).
> > > 
> > > This sounds like, "We don't like the kernel development model, it
> > > requires us to break everything up into small patches and show our work
> > > to everyone."
> > 
> > It's not the kernel development mode, it's something that you try to
> > impose on staging users.  Creating a more less new driver from a pig
> > pile of junk doesn't make sense minimized patches with micro-review.
> 
> I tried it once on ath6kl.
> Ended up with ~300 patches and a very clean tree.

Yes it can be done, and others do it for other drivers, it just takes a
bit of effort.

But as this is pretty much a total rewrite, that's fine to do
out-of-tree and then just submit it for inclusion in the main wireless
tree and then we can delete this driver.

We've done this for other wireless drivers, so it wouldn't be the first
time.  I just wanted to make sure that it wasn't something with how the
overall development process was working that caused this.

thanks,

greg k-h

Re: Plan for ath6kl cleanup

[Dan Carpenter]

On 5/20/11, Kalle Valo <kalle.valo@atheros.com> wrote:
>> Don't worry about "cluttering" up the mailing list, send them to me, and
>> to the driver devel mailing list, and I will take them just fine.
>
> I'm talking about linux-wireless mailing list. And yes, I really think
> that sending patches like removing A_MEMSET() is cluttering the mailing
> list. I would rather see wireless hackers doing something productive
> than reading patches like that.
>

Mutt has a short cut to delete a whole thread.  You learn that
quickly enough when you deal with Greg KH.

Anyway, those patches aren't hard to review.  I use a script
for it:
http://marc.info/?l=linux-driver-devel&m=129699522622014&w=2
It strips out the whitespace changes and the pure sed work
and it only shows interesting changes.  You can invoke it
directly from mutt so it takes about 30 seconds to look over
each patch.

I'm not saying that everything should get sent to lkml, but
those patches are fine on the driver devel list.  The rule that
everything should go to a public mailing list is a good one.

regards,
dan carpenter