Re: [PATCH v2] staging: vc04_services: rework ioctl code path

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

 



On Tue, 2016-11-15 at 10:55 +0100, Greg KH wrote:
> On Thu, Nov 10, 2016 at 10:15:31PM -0800, Michael Zoran wrote:
> > VCHIQ/vc04_services has a userland device interface
> > that includes ioctls. The current ioctl implementation
> > is a single monolithic function over 1,000+ lines
> > that handles 17 different ioctls through a complex
> > maze of switch and if statements.
> > 
> > The change reimplements that code path by breaking
> > up the code into smaller, easier to maintain functions
> > and uses a dispatch table to invoke the correct function.
> > 
> > Testing:
> > 
> > 1. vchiq_test -f 10 and vchiq_test -p 1 were run from a native
> > 64-bit OS(debian sid).
> > 
> > 2. vchiq_test -f 10 and vchiq_test -p 1 where run from a 32-bit
> > chroot install from the same OS.
> > 
> > Both test cases pass.
> > 
> > This is V2 of this patch.  Changes include:
> > 
> > 1. More code has been moved to the dispatch routine.
> > The dispatch routine is now responsible for copying the top-level
> > data into and out of kernel space by using the data encoded in
> > the ioctl command number.
> > 
> > 2. The number of parameters have been reduced for the handling
> > functions by giving a different prototype to ioctls that pass
> > no arguments.
> > 
> > 3. Macros in linux/compat.h are now used for compatibility data
> > structures.
> > 
> > Signed-off-by: Michael Zoran <mzoran@xxxxxxxxxxxx>
> > ---
> >  .../vc04_services/interface/vchiq_arm/vchiq_arm.c  | 1733
> > +++++++++++++-------
> >  .../interface/vchiq_arm/vchiq_ioctl.h              |   96 ++
> >  2 files changed, 1200 insertions(+), 629 deletions(-)
> 
> This is a rough patch to review, can you break this up into a patch
> series that moves each ioctl to a separate function as needed?
> 
> thanks,
> 
> greg k-h

I'll see what I can do, but since this is getting closer to the
holidays I'm planing to spend less time on this. Perhaps someone else
may be interested in cleaning up the ioctl path?

Since the old code uses obsolete kernel features, it's going to take
some work to fix without adding any checkpatch.pl warnings.
_______________________________________________
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