Hi Brian,

On Mon, Mar 15, 2010 at 21:20:13, Kevin Hilman wrote:
> Brian Niebuhr <bnieb...@efjohnson.com> writes:
> 
> >  > > This patch is a significant overhaul of the davinci spi 
> >> controller driver
> >> > that corrects multiple errors:
> >> >
> >> > - Eliminate a race condition that exists for slow SPI devices
> >> > - Fix DMA transfer length error
> >> > - Fix limitations preventing multiple SPI devices on the 
> >> same controller
> >> >
> >> > Signed-off-by: Brian Niebuhr <bnieb...@efjohnson.com>
> >> 
> >> The verbose description of the issues addressed from PATCH 0/2 should
> >> go here is well so it makes it into the permanent git history.
> >
> > I can certainly do that.
> >  
> >> That being said, I think for the sake of reviewing, you're going to
> >> need to break this up into reviewable pieces, each having a verbose
> >> description of the issues being solved.
> >> 
> >> There is also a mixture of fixes, enhancements, renames etc.  These
> >> should be done as separate patches. 
> >> 
> >> I know that it's more work to break it up like this, but that's the
> >> only way to make a large change like this reviewable by others.
> >
> > I guess I was hoping that this could be reviewed as if it were a new
> > driver submission.  I ended up more or less rewriting all of the
> > functional parts of the driver (txrx_bufs(), chipselect(), IRQs and DMA
> > callbacks), so it's very difficult to show this as a series of changes.
> > I do understand the problem from your perspective, though.  My thought
> > was that if the TI folks were willing to look the driver over and they
> > gave their blessing, that you would look at it as if it were a
> > replacement driver and accept or deny it on that basis.
> 
> I'm OK with the approach of considering it as a brand new driver.  The
> changelog made me think it was a bunch of fixes/enhancements and not a
> re-write.
> 
> It should then be made more clear in the changelog that this is
> essentially a re-write, and why it is not done in a series of small
> changes.
> 
> Whichever approach, this will need to worked out between you and the
> origial TI authors (Sandeep, Sudahkar) who will need to review/signoff
> this replacement.
> 

I tried testing this patch on some DA8xx platforms I have but could not do it
as the patch did not apply cleanly to Kevin's tree. You might have to rebase
this patch so that it applies on Kevin's tree.

Also, I would still prefer if you can breakdown this patch as per the various
fixes you have done because the current SPI driver is already accepted in 
mainline
and is working, of course with some limitations. But if you want to treat it as
complete overhaul, then it is better to split the patch such that, one patch 
removes
the existing driver and other patch adds the new driver. In this way it is easy 
to
review the patch.

Regards,
Sudhakar


_______________________________________________
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source

Reply via email to