Re: [PATCH 126/141] staging: unisys: Convert the epilog functions to visor_device

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

 



On Fri, May 08, 2015 at 12:06:37PM -0400, Don Zickus wrote:
> On Fri, May 08, 2015 at 04:02:06PM +0200, Greg KH wrote:
> > On Fri, May 08, 2015 at 09:56:41AM -0400, Don Zickus wrote:
> > > On Fri, May 08, 2015 at 11:00:18AM +0300, Dan Carpenter wrote:
> > > > On Tue, May 05, 2015 at 06:37:43PM -0400, Benjamin Romer wrote:
> > > > > From: Don Zickus <dzickus@xxxxxxxxxx>
> > > > > @@ -1128,7 +1119,7 @@ device_epilog(u32 bus_no, u32 dev_no, struct spar_segment_state state, u32 cmd,
> > > > >  		switch (cmd) {
> > > > >  		case CONTROLVM_DEVICE_CREATE:
> > > > >  			if (notifiers->device_create) {
> > > > > -				(*notifiers->device_create) (bus_no, dev_no);
> > > > > +				(*notifiers->device_create) (dev_info);
> > > > >  				notified = true;
> > > > >  			}
> > > > >  			break;
> > > > 
> > > > This doesn't work.  We don't change the (*notifiers->device_create)
> > > > function pointer until the next patch.
> > > 
> > > Correct.  The changes were so intertwined that I could either post a giant
> > > monolithic patch and maintain bisectability or break things into smaller
> > > chunks for review purpose but lose the ability to bisect.
> > > 
> > > I chose the later.  The weak argument we used is that no one will enable
> > > this driver (except unisys) so it normally won't break bisecting by others.
> > 
> > Sorry, but no, you can't break the build at any point in any patch.
> 
> Alright, then I will ask Ben to fold the 5 patches together. :-(

How about reworking them in a different way?  There's lots of ways to
evolve code that doesn't require folding things all together.

Yes, it takes more work, but it makes reviewing stuff easier, which is
the most important thing here.

greg k-h
_______________________________________________
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