Re: [PATCH 3/4] staging: ks7010: add initial cfg80211 implementation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

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



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux