Re: [PATCH v2] SPI: DaVinci: Adding SPI driver for DaVinci

[Kevin Hilman <khilman@deeprootsystems.com>]

"Paulraj, Sandeep" <s-paulraj@ti.com> writes:

> Kevin,
>
> Please see inline
>
>> 
>> > The patch has received no comments so far (here and on spi-general-
>> devel).
>> >
>> > Can someone test it on davinci's other that the DM6446 to see that
>> support for
>> > others has not broken?
>> >
>> > Kevin - Is there anything that keeps it from merging upstream to this
>> tree?
>> 
>> Hi Pablo,
>> 
>> Sorry for the delay, I've been travelling and not able to watch
>> DaVinci closely enough...
>> 
>> This driver should be merged via the SPI subsystem (maintained by
>> David Brownell), not the Davinci core code which I maintain.
>> 
>> That being said, in my view, here's why this driver is not ready for
>> upstream:
>> 
>> 1) The original driver from Sandeep that you based yours on was still
>>    going through revisions.  The last review comments[1] from David
>>    Brownell had not yet been addressed by Sandeep.  I hope that
>>    Sandeep will have a chance to address the existing review comments
>>    on his code, and then review yours.  However, you've made it
>>    rather difficult to do that because...
>
> [Sandeep] There were a set of comments from David Brownell(which was
> actually, thanks to him, in the from of a patch).  David did say
> that the SPI support in that form was ready for an initial merge. I
> tested it on DM355/Dm365 and Dm6467 and that driver(meant for the
> initial merge) is in our ARAGO tree.  Afcourse we all agreed that
> there are things to add in the SPI driver.  Also IIRC(and I am
> willing to be corrected) David did say that he would send it
> upstream when he got some time so I did not do it myself. The fact
> that he maintains the SPI subsystem had a part to play in my
> decision.

OK, I'll continue the original thread with some more questions on this
topic.

>> 
>> 2) You should have your patch apply on top of Sandeep's series, not
>>    just absorb it.  That way we can clearly see what you are adding
>>    and/or changing from Sandeeps original driver.  To make this part
>>    easier, I created a 'temp/spi' branch of davinci git where I've
>>    pushed the latest versions of the patches from Sandeep.  Any
>>    additions/updates/fixes you have should be posted as patches
>>    against that for easier discussion and review.
>> 
>> 3) As Sandeep did, you should keep the changes to the board/SoC code
>>    (arch/arm/mach-davinci/*) as a separate patch from the driver code
>>    (drivers/spi/*)
>> 
>> 4) this driver needs more testing
>
> [Sandeep] tested by TI test team on DM355 and DM365 and I have tested on DM6467.

Sandeep, your driver recieved sufficient testing.  The driver that I
was saying needs more changes was this one proposed by Pablo.

Kevin