On Thu, Jun 15, 2017 at 09:59:06AM +1000, Tobin C. Harding wrote:
> On Wed, Jun 14, 2017 at 12:24:21PM +0200, Greg Kroah-Hartman wrote:
> > On Wed, Jun 14, 2017 at 04:30:37PM +1000, Tobin C. Harding wrote:
> > > Currently we are in the process of replacing the WEXT interface with
> > > the cfg80211 API. WEXT code is currently within a sub directory and
> > > not included in the module build.
> > >
> > > The driver is designed with various layers of abstraction;
> > >
> > > - The main layer contains the net_device_ops callbacks. Also included
> > > in this layer is the rx/tx code.
> > > - The SDIO layer contains code specific to the SDIO hardware.
> > > - The cfg80211 layer contains the implementation of the cfg80211
> > > configuration API.
> > > - The HIF (Host InterFace) layer provides driver policy and interfaces
> > > between the cfg80211 layer and the FIL.
> > > - The FIL (Firmware Interface Layer) provides mechanism for
> > > interfacing with the firmware.
> > >
> > > The firmware interface is derived from the WEXT driver, if we
> > > write code to interface with the firmware as a separate abstraction
> > > layer then the cfg80211 code and the rest of the driver functionality
> > > can be written cleanly from scratch.
> > >
> > > The separate layers are restricted to calls as indicated by the
> > > following diagram, A --> B indicates layer A calls functions in layer B.
> > >
> > > ----> main (tx/rx) <---> SDIO
> > > |
> > > | |
> > > | |
> > > v v
> > >
> > > cfg80211 <---> HIF
> > >
> > > |
> > > |
> > > v
> > >
> > > FIL
> > >
> > > Implementation status is outlined in README.rst. A todo list is
> > > included in TODO.rst.
> > >
> > > Add initial cfg80211 implementation.
> > >
> > > Signed-off-by: Tobin C. Harding <me@xxxxxxxx>
> > > ---
> > > drivers/staging/ks7010/Kconfig | 2 +
> > > drivers/staging/ks7010/Makefile | 28 +-
> > > drivers/staging/ks7010/README.rst | 91 +++
> > > drivers/staging/ks7010/TODO.rst | 30 +
> > > drivers/staging/ks7010/cfg80211.c | 981 +++++++++++++++++++++++++++
> > > drivers/staging/ks7010/cfg80211.h | 40 ++
> > > drivers/staging/ks7010/common.h | 33 +
> > > drivers/staging/ks7010/eap.h | 73 ++
> > > drivers/staging/ks7010/fil.c | 1294 ++++++++++++++++++++++++++++++++++++
> > > drivers/staging/ks7010/fil.h | 559 ++++++++++++++++
> > > drivers/staging/ks7010/fil_types.h | 851 ++++++++++++++++++++++++
> > > drivers/staging/ks7010/hif.c | 505 ++++++++++++++
> > > drivers/staging/ks7010/hif.h | 202 ++++++
> > > drivers/staging/ks7010/ks7010.h | 309 +++++++++
> > > drivers/staging/ks7010/main.c | 337 ++++++++++
> > > drivers/staging/ks7010/rx.c | 130 ++++
> > > drivers/staging/ks7010/sdio.c | 691 +++++++++++++++++++
> > > drivers/staging/ks7010/sdio.h | 37 ++
> > > drivers/staging/ks7010/tx.c | 170 +++++
> > > 19 files changed, 6362 insertions(+), 1 deletion(-)
> > > create mode 100644 drivers/staging/ks7010/README.rst
> > > create mode 100644 drivers/staging/ks7010/TODO.rst
> > > create mode 100644 drivers/staging/ks7010/cfg80211.c
> > > create mode 100644 drivers/staging/ks7010/cfg80211.h
> > > create mode 100644 drivers/staging/ks7010/common.h
> > > create mode 100644 drivers/staging/ks7010/eap.h
> > > create mode 100644 drivers/staging/ks7010/fil.c
> > > create mode 100644 drivers/staging/ks7010/fil.h
> > > create mode 100644 drivers/staging/ks7010/fil_types.h
> > > create mode 100644 drivers/staging/ks7010/hif.c
> > > create mode 100644 drivers/staging/ks7010/hif.h
> > > create mode 100644 drivers/staging/ks7010/ks7010.h
> > > create mode 100644 drivers/staging/ks7010/main.c
> > > create mode 100644 drivers/staging/ks7010/rx.c
> > > create mode 100644 drivers/staging/ks7010/sdio.c
> > > create mode 100644 drivers/staging/ks7010/sdio.h
> > > create mode 100644 drivers/staging/ks7010/tx.c
> > >
> > > diff --git a/drivers/staging/ks7010/Kconfig b/drivers/staging/ks7010/Kconfig
> > > index 437b928..71f2026 100644
> > > --- a/drivers/staging/ks7010/Kconfig
> > > +++ b/drivers/staging/ks7010/Kconfig
> > > @@ -1,5 +1,7 @@
> > > config KS7010
> > > tristate "KeyStream KS7010 SDIO support"
> > > + depends on CFG80211
> > > + depends on MMC
> >
> > tabs vs. spaces???
>
> Face palm.
>
> > > ---help---
> > > This is a driver for KeyStream KS7010 based SDIO WIFI cards. It is
> > > found on at least later Spectec SDW-821 (FCC-ID "S2Y-WLAN-11G-K" only,
> > > diff --git a/drivers/staging/ks7010/Makefile b/drivers/staging/ks7010/Makefile
> > > index 9444885..1cd570e 100644
> > > --- a/drivers/staging/ks7010/Makefile
> > > +++ b/drivers/staging/ks7010/Makefile
> > > @@ -1 +1,27 @@
> > > -# Makefile intentionally left blank
> > > +#-------------------------------------------------------------------------
> > > +# Driver for KeyStream wireless LAN cards.
> > > +#
> > > +# Copyright (C) 2005-2008 KeyStream Corp.
> > > +# Copyright (C) 2009 Renesas Technology Corp.
> > > +# Copyright (C) 2017 Tobin C. Harding.
> > > +#
> > > +# This program is free software; you can redistribute it and/or modify
> > > +# it under the terms of the GNU General Public License version 2 as
> > > +# published by the Free Software Foundation.
> > > +#
> > > +# This program is distributed in the hope that it will be useful, but
> > > +# WITHOUT ANY WARRANTY; without even the implied warranty of
> > > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > > +# General Public License for more details.
> >
> > It's a makefile, not a "program", so why is all of this here?
>
> I followed ath6kl for much of the time while writing this, ath6kl does
> this. I will remove it and resubmit.
>
> > > +#-------------------------------------------------------------------------
> > > +
> > > +ccflags-y += -DDEBUG
> > > +
> > > +obj-$(CONFIG_KS7010) += ks7010.o
> > > +ks7010-y += main.o
> > > +ks7010-y += tx.o
> > > +ks7010-y += rx.o
> > > +ks7010-y += sdio.o
> > > +ks7010-y += cfg80211.o
> > > +ks7010-y += fil.o
> > > +ks7010-y += hif.o
> >
> > Wait, is this a whole new driver for this device? Why not just put it
> > in a new directory and just forget about the old one entirely?
>
> Thanks for your responses Greg. I do not know the correct protocol to
> follow for what I am attempting with this patch set. I mentioned this in
> the RFC hoping to get some direction. There were no comments so I
> proceeded as I had done in the RFC;
>
> 1. Move WEXT code to sub-directory.
> 2. Add new driver code to main ks7010 directory.
> 3. Add maintainers entry.
Response from kbuild test robot implies that this is not the best way
to go about it.
Note: the linux-review/Tobin-C-Harding/staging-ks7010-cfg80211-conversion/20170614-224320
HEAD 2b0460aebae71f28fdc7ab54b37af26fca3e4f4e builds fine.
It only hurts bisectibility.
All errors (new ones prefixed by >>):
>> ld: cannot find drivers/staging/ks7010/built-in.o: No such file or directory
Better method happily followed if suggested.
> Following my understanding of kernel patching process the driver had
> to build with each patch applied, therefore after moving the WEXT code
> I left a blank Makefile and removed the dependencies from Kconfig
>
>
> The WEXT to cfg80211 conversion is basically a complete re-write of
> the driver. Does this mean it is the same driver or a new one, I don't
> know? The WEXT code is still needed for reference, we have only the
> WEXT driver to base the cfg80211 driver on. I know that it is in the git
> history and I could of clobbered the whole thing but I thought it more
> informative to keep the WEXT code in a sub directory to make explicit
> that the cfg80211 driver was being written based off of the WEXT
> driver.
>
> > And testing it would be good to do :)
>
> I'm working on the testing. Currently I have the card to test but not
> a machine that supports SDIO with a vanilla kernel. As such I have not
> managed to test the WEXT code either.
>
> This patch set does not implement a complete functioning driver. I
> have submitted the driver as is in an attempt to get input from the
> kernel community. Failing that, if the driver is in staging as is,
> then I am forced to be follow a more meticulous development
> methodology instead of wildly hacking as I have done for the last
> month. With my minimal knowledge, I believe the design is stable
> enough to enable this. I am happy to be corrected if I am wrong. I am
> very much a learner, I would like to follow [learn] the correct kernel
> development methods.
>
> thanks,
> Tobin.
> _______________________________________________
> devel mailing list
> devel@xxxxxxxxxxxxxxxxxxxxxx
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
