Re: [PATCH 0/11] convert d80211 to a proper protocol

[Johannes Berg <johannes@sipsolutions.net>]

[-- Attachment #1: Type: text/plain, Size: 3354 bytes --]

[reordering a bit]

> >    This changes the 'cookie' that d80211 returns from alloc_hw
> >    to be an opaque value to the driver. Turned out that it wasn't
> >    such a great idea but since it was generally a clean up I kept
> >    this patch to base my other patches on.
> 
> ACK.

> What did happen with
> d80211: add a function to get the wiphy index
> d80211: add a perm_addr hardware property
> d80211: add a struct device* hardware property
> d80211: add a ethtool_ops hardware property
> patches?

Well after some chat with a few people I decided that it was stupid and
not very maintainable to copy all the fields in net_device to a new
structure.

> > 009-d80211-convert-spaces.patch
> >    d80211: convert leading spaces to tabs
> > 
> >    I hated working on the code, so I did this. The next patch
> >    breaks everything anyway.
> 
> NAK. There are too many patches pending. Let's do this just before
> merging.

Oh come off it! It's really stupid to have to check all the tabs/spaces
all the time. The patch changes 451 lines. And wiggle can handle that
just fine. Besides, if you do
s/^\+       /+\t/
s/^-       /-\t/
s/        /\t/
on your patches, they'll be fine too.

> This is too big patch for a review, 

Yeah. It's pretty bad actually, but I couldn't really find a good way to
split it into logical chunks.

> > * The mdev no longer has a sub_if_data attached (why ever did it??)
> >   It's private area is for the driver since we don't create it but
> >   the driver does. I did keep the notation of mdev/master all through,
> >   but it's no longer the stacks device. Keep that in mind.
> 
> This definitely breaks AP mode. In the code, there is heavily (ab)used
> the fact that the master device is in fact an AP device. I tried to fix
> that but it was so difficult I gave up. It is needed to rewrite the
> whole RX path (and even that is probably not enough).

Bugger. I didn't notice that. I'll have a look. That is indeed a
showstopper.

> As this will
> be fixed for free when we have native 802.11 devices, I don't think we
> need to do anything about it now.

I don't think I understand this. I mean, my patch actually gives us
native 802.11 devices by making the drivers register those and then
handling them virtually similar to how 8021q handles ethernet devices. I
honestly thought that this was the plan for said "native 802.11
devices".

> > * sysfs layout changed. There is no wiphy or an ieee80211 class any more,
> >   the attributes that used to be there are now in the net_device that
> >   the driver registered, and our attributes are below the devices we created.
> 
> You want an ieee80211 class. Once you get rid of a master interface you
> need something with per-hardware information, statistics etc.

Yeah, I gave up trying to get rid of the master interface in favour of
having a native 802.11 device which is registered by the phy driver
instead.

> > * sysfs layout changed. There is no wiphy or an ieee80211 class any more,
> >   the attributes that used to be there are now in the net_device that
> >   the driver registered, and our attributes are below the devices we created.
> 
> Doesn't belong to this patch.

Had to be here initially due to the way I did things, but ok, probably
changeable.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]