Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1467373 > unrolled thread

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

Started by"Sell, Timothy C" <Timothy.Sell@unisys.com>
First post2016-08-22 07:30 +0200
Last post2016-08-22 16:20 +0200
Articles 5 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  RE: [PATCH 3/3] drivers: Add visorbus to the drivers/virt directory "Sell, Timothy C" <Timothy.Sell@unisys.com> - 2016-08-22 07:30 +0200
    Re: [PATCH 3/3] drivers: Add visorbus to the drivers/virt directory 'Greg KH' <gregkh@linuxfoundation.org> - 2016-08-22 10:20 +0200
      RE: [PATCH 3/3] drivers: Add visorbus to the drivers/virt directory "Sell, Timothy C" <Timothy.Sell@unisys.com> - 2016-08-22 12:50 +0200
        Re: [PATCH 3/3] drivers: Add visorbus to the drivers/virt directory 'Greg KH' <gregkh@linuxfoundation.org> - 2016-08-22 16:10 +0200
          Re: [PATCH 3/3] drivers: Add visorbus to the drivers/virt directory 'Greg KH' <gregkh@linuxfoundation.org> - 2016-08-22 16:20 +0200

#1467373 — RE: [PATCH 3/3] drivers: Add visorbus to the drivers/virt directory

From"Sell, Timothy C" <Timothy.Sell@unisys.com>
Date2016-08-22 07:30 +0200
SubjectRE: [PATCH 3/3] drivers: Add visorbus to the drivers/virt directory
Message-ID<s8PY5-22v-3@gated-at.bofh.it>
> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Sunday, August 21, 2016 2:05 PM
> To: Kershner, David A <David.Kershner@unisys.com>
> Cc: corbet@lwn.net; tglx@linutronix.de; mingo@redhat.com;
> hpa@zytor.com; Arfvidson, Erik <Erik.Arfvidson@unisys.com>; Sell, Timothy
> C <Timothy.Sell@unisys.com>; hofrat@osadl.org; dzickus@redhat.com;
> jes.sorensen@redhat.com; Curtin, Alexander Paul
> <Alexander.Curtin@unisys.com>; janani.rvchndrn@gmail.com;
> sudipm.mukherjee@gmail.com; prarit@redhat.com; Binder, David Anthony
> <David.Binder@unisys.com>; nhorman@redhat.com;
> dan.j.williams@intel.com; linux-kernel@vger.kernel.org; linux-
> doc@vger.kernel.org; driverdev-devel@linuxdriverproject.org; *S-Par-
> Maintainer <SParMaintainer@unisys.com>
> 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@unisys.com>
> > Reviewed-by: Tim Sell <Timothy.Sell@unisys.com>
> > ---
> >  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)
{
	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
controlvm_respond(struct controlvm_message_header *msg_hdr, int response)
{
	struct controlvm_message outmsg;

	controlvm_init_response(&outmsg, msg_hdr, response);
	if (outmsg.hdr.flags.test_message == 1)
		return;

	if (!visorchannel_signalinsert(controlvm_channel,
				       CONTROLVM_QUEUE_REQUEST, &outmsg)) {
		return;
	}
}

One (or both) of us is confused.

Tim Sell

[toc] | [next] | [standalone]


#1467442

From'Greg KH' <gregkh@linuxfoundation.org>
Date2016-08-22 10:20 +0200
Message-ID<s8SCB-3NU-7@gated-at.bofh.it>
In reply to#1467373
On Mon, Aug 22, 2016 at 02:48:18AM +0000, Sell, Timothy C wrote:
> > -----Original Message-----
> > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > Sent: Sunday, August 21, 2016 2:05 PM
> > To: Kershner, David A <David.Kershner@unisys.com>
> > Cc: corbet@lwn.net; tglx@linutronix.de; mingo@redhat.com;
> > hpa@zytor.com; Arfvidson, Erik <Erik.Arfvidson@unisys.com>; Sell, Timothy
> > C <Timothy.Sell@unisys.com>; hofrat@osadl.org; dzickus@redhat.com;
> > jes.sorensen@redhat.com; Curtin, Alexander Paul
> > <Alexander.Curtin@unisys.com>; janani.rvchndrn@gmail.com;
> > sudipm.mukherjee@gmail.com; prarit@redhat.com; Binder, David Anthony
> > <David.Binder@unisys.com>; nhorman@redhat.com;
> > dan.j.williams@intel.com; linux-kernel@vger.kernel.org; linux-
> > doc@vger.kernel.org; driverdev-devel@linuxdriverproject.org; *S-Par-
> > Maintainer <SParMaintainer@unisys.com>
> > 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@unisys.com>
> > > Reviewed-by: Tim Sell <Timothy.Sell@unisys.com>
> > > ---
> > >  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

[toc] | [prev] | [next] | [standalone]


#1467540

From"Sell, Timothy C" <Timothy.Sell@unisys.com>
Date2016-08-22 12:50 +0200
Message-ID<s8UXL-5aa-7@gated-at.bofh.it>
In reply to#1467442
> -----Original Message-----
> From: 'Greg KH' [mailto:gregkh@linuxfoundation.org]
> Sent: Monday, August 22, 2016 4:16 AM
> To: Sell, Timothy C <Timothy.Sell@unisys.com>
> Cc: corbet@lwn.net; tglx@linutronix.de; mingo@redhat.com;
> hpa@zytor.com; Arfvidson, Erik <Erik.Arfvidson@unisys.com>;
> hofrat@osadl.org; dzickus@redhat.com; jes.sorensen@redhat.com; Curtin,
> Alexander Paul <Alexander.Curtin@unisys.com>;
> janani.rvchndrn@gmail.com; sudipm.mukherjee@gmail.com;
> prarit@redhat.com; Binder, David Anthony <David.Binder@unisys.com>;
> nhorman@redhat.com; dan.j.williams@intel.com; linux-
> kernel@vger.kernel.org; linux-doc@vger.kernel.org; driverdev-
> devel@linuxdriverproject.org; *S-Par-Maintainer
> <SParMaintainer@unisys.com>; Kershner, David A
> <David.Kershner@unisys.com>
> Subject: Re: [PATCH 3/3] drivers: Add visorbus to the drivers/virt directory
> 
> 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?

This particular comment happened to affect the credibility of all the other
ones, so it needed to be addressed first!

Tim Sell

> 
> thanks,
> 
> greg k-h

[toc] | [prev] | [next] | [standalone]


#1467656

From'Greg KH' <gregkh@linuxfoundation.org>
Date2016-08-22 16:10 +0200
Message-ID<s8Y5k-7jf-21@gated-at.bofh.it>
In reply to#1467540
On Mon, Aug 22, 2016 at 10:46:10AM +0000, Sell, Timothy C wrote:
> > -----Original Message-----
> > From: 'Greg KH' [mailto:gregkh@linuxfoundation.org]
> > Sent: Monday, August 22, 2016 4:16 AM
> > To: Sell, Timothy C <Timothy.Sell@unisys.com>
> > Cc: corbet@lwn.net; tglx@linutronix.de; mingo@redhat.com;
> > hpa@zytor.com; Arfvidson, Erik <Erik.Arfvidson@unisys.com>;
> > hofrat@osadl.org; dzickus@redhat.com; jes.sorensen@redhat.com; Curtin,
> > Alexander Paul <Alexander.Curtin@unisys.com>;
> > janani.rvchndrn@gmail.com; sudipm.mukherjee@gmail.com;
> > prarit@redhat.com; Binder, David Anthony <David.Binder@unisys.com>;
> > nhorman@redhat.com; dan.j.williams@intel.com; linux-
> > kernel@vger.kernel.org; linux-doc@vger.kernel.org; driverdev-
> > devel@linuxdriverproject.org; *S-Par-Maintainer
> > <SParMaintainer@unisys.com>; Kershner, David A
> > <David.Kershner@unisys.com>
> > Subject: Re: [PATCH 3/3] drivers: Add visorbus to the drivers/virt directory
> > 
> > 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?
> 
> This particular comment happened to affect the credibility of all the other
> ones, so it needed to be addressed first!

Oh, so everything else was credible?  Glad to hear it :)

greg k-h

[toc] | [prev] | [next] | [standalone]


#1467664

From'Greg KH' <gregkh@linuxfoundation.org>
Date2016-08-22 16:20 +0200
Message-ID<s8YeZ-7mP-11@gated-at.bofh.it>
In reply to#1467656
On Mon, Aug 22, 2016 at 10:02:43AM -0400, 'Greg KH' wrote:
> On Mon, Aug 22, 2016 at 10:46:10AM +0000, Sell, Timothy C wrote:
> > > -----Original Message-----
> > > From: 'Greg KH' [mailto:gregkh@linuxfoundation.org]
> > > Sent: Monday, August 22, 2016 4:16 AM
> > > To: Sell, Timothy C <Timothy.Sell@unisys.com>
> > > Cc: corbet@lwn.net; tglx@linutronix.de; mingo@redhat.com;
> > > hpa@zytor.com; Arfvidson, Erik <Erik.Arfvidson@unisys.com>;
> > > hofrat@osadl.org; dzickus@redhat.com; jes.sorensen@redhat.com; Curtin,
> > > Alexander Paul <Alexander.Curtin@unisys.com>;
> > > janani.rvchndrn@gmail.com; sudipm.mukherjee@gmail.com;
> > > prarit@redhat.com; Binder, David Anthony <David.Binder@unisys.com>;
> > > nhorman@redhat.com; dan.j.williams@intel.com; linux-
> > > kernel@vger.kernel.org; linux-doc@vger.kernel.org; driverdev-
> > > devel@linuxdriverproject.org; *S-Par-Maintainer
> > > <SParMaintainer@unisys.com>; Kershner, David A
> > > <David.Kershner@unisys.com>
> > > Subject: Re: [PATCH 3/3] drivers: Add visorbus to the drivers/virt directory
> > > 
> > > 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?
> > 
> > This particular comment happened to affect the credibility of all the other
> > ones, so it needed to be addressed first!
> 
> Oh, so everything else was credible?  Glad to hear it :)

And please, fix your quoting, you quote all of the cc: lines, yet none
of the text?  Odd emailer...

greg k-h

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web