Re: [PATCH 3/3] drivers: Add visorbus to the drivers/virt directory

['Greg KH']

On Mon, Aug 22, 2016 at 02:48:18AM +0000, Sell, Timothy C wrote:
> > -----Original Message-----
> > From: Greg KH [mailto:gregkh@xxxxxxxxxxxxxxxxxxx]
> > Sent: Sunday, August 21, 2016 2:05 PM
> > To: Kershner, David A <David.Kershner@xxxxxxxxxx>
> > Cc: corbet@xxxxxxx; tglx@xxxxxxxxxxxxx; mingo@xxxxxxxxxx;
> > hpa@xxxxxxxxx; Arfvidson, Erik <Erik.Arfvidson@xxxxxxxxxx>; Sell, Timothy
> > C <Timothy.Sell@xxxxxxxxxx>; hofrat@xxxxxxxxx; dzickus@xxxxxxxxxx;
> > jes.sorensen@xxxxxxxxxx; Curtin, Alexander Paul
> > <Alexander.Curtin@xxxxxxxxxx>; janani.rvchndrn@xxxxxxxxx;
> > sudipm.mukherjee@xxxxxxxxx; prarit@xxxxxxxxxx; Binder, David Anthony
> > <David.Binder@xxxxxxxxxx>; nhorman@xxxxxxxxxx;
> > dan.j.williams@xxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-
> > doc@xxxxxxxxxxxxxxx; driverdev-devel@xxxxxxxxxxxxxxxxxxxxxx; *S-Par-
> > Maintainer <SParMaintainer@xxxxxxxxxx>
> > Subject: Re: [PATCH 3/3] drivers: Add visorbus to the drivers/virt directory
> > 
> > On Fri, Jun 10, 2016 at 11:23:56PM -0400, David Kershner wrote:
> > > visorbus is currently located at drivers/staging/visorbus,
> > > this patch moves it to drivers/virt.
> > >
> > > Signed-off-by: David Kershner <david.kershner@xxxxxxxxxx>
> > > Reviewed-by: Tim Sell <Timothy.Sell@xxxxxxxxxx>
> > > ---
> > >  drivers/staging/unisys/Kconfig                                        | 3 +--
> > >  drivers/staging/unisys/Makefile                                       | 1 -
> > >  drivers/virt/Kconfig                                                  | 2 ++
> > >  drivers/virt/Makefile                                                 | 1 +
> > >  drivers/{staging/unisys => virt}/visorbus/Kconfig                     | 0
> > >  drivers/{staging/unisys => virt}/visorbus/Makefile                    | 0
> > >  drivers/{staging/unisys => virt}/visorbus/controlvmchannel.h          | 0
> > >  drivers/{staging/unisys => virt}/visorbus/controlvmcompletionstatus.h | 0
> > >  drivers/{staging/unisys => virt}/visorbus/iovmcall_gnuc.h             | 0
> > >  drivers/{staging/unisys => virt}/visorbus/vbuschannel.h               | 0
> > >  drivers/{staging/unisys => virt}/visorbus/vbusdeviceinfo.h            | 0
> > >  drivers/{staging/unisys => virt}/visorbus/vbushelper.h                | 0
> > >  drivers/{staging/unisys => virt}/visorbus/visorbus_main.c             | 0
> > >  drivers/{staging/unisys => virt}/visorbus/visorbus_private.h          | 0
> > >  drivers/{staging/unisys => virt}/visorbus/visorchannel.c              | 0
> > >  drivers/{staging/unisys => virt}/visorbus/visorchipset.c              | 0
> > 
> > I picked one random file here, this last one, and found a number of
> > "odd" things in it.
> > 
> > So, given that I can't really comment on the patch itself, I'm going to
> > include the file below, quote it, and then provide some comments, ok?
> > > /* visorchipset_main.c
> > > ...
> > > static void
> > > controlvm_init_response(struct controlvm_message *msg,
> > > 			struct controlvm_message_header *msg_hdr, int response)
> > > {
> > > 	memset(msg, 0, sizeof(struct controlvm_message));
> > > 	memcpy(&msg->hdr, msg_hdr, sizeof(struct controlvm_message_header));
> > > 	msg->hdr.payload_bytes = 0;
> > > 	msg->hdr.payload_vm_offset = 0;
> > > 	msg->hdr.payload_max_bytes = 0;
> > > 	if (response < 0) {
> > > 		msg->hdr.flags.failed = 1;
> > > 		msg->hdr.completion_status = (u32)(-response);
> > > 	}
> > > }
> > > 
> > > static void
> > > Billy(struct controlvm_message_header *msg_hdr, int response)
> > 
> > Not John?  Bob?  Sally?  Alice?  Bernise?  Jean?  Molly?
> 
> Huh? What version of source code are you looking at??
> 
> The file being moved by this patch should be
> drivers/staging/unisys/visorbus/visorchipset.c in your staging-next branch  
> (https://git.kernel.org/cgit/linux/kernel/git/gregkh/staging.git/tree/drivers/staging/unisys/visorbus/visorchipset.c?h=staging-next).
> If you look there, you will NOT find "Billy()", but instead "controlvm_respond()", i.e.:
> 
> static void
> controlvm_init_response(struct controlvm_message *msg,
> 			struct controlvm_message_header *msg_hdr, int response)


oh that's funny, I have no idea what happened here, perhaps when I
copied the file into the original email?  Strange things happen while on
planes...

Anyway, ok, one comment addressed, great!  Now about the many many
others?

thanks,

greg k-h