[U-Boot] [PATCH v3 5/5] ARM: dm: spi: Add support DM/DTS for i.MX28 mxs SPI driver (DM_SPI conversion)

[Marek Vasut]

On 6/17/19 2:27 PM, Lukasz Majewski wrote:
> Hi Marek,
> 
>> On 6/17/19 8:49 AM, Lukasz Majewski wrote:
>>> Hi Marek,
>>>   
>>>> On 6/16/19 12:34 AM, Lukasz Majewski wrote:  
>>>>> This commit converts mxs_spi driver to support DM/DTS.
>>>>>
>>>>> Signed-off-by: Lukasz Majewski <lukma at denx.de>    
>>>>
>>>> Is the non-DM part needed for anything ?   
>>>
>>> Do you mean the non-DM part of the mxs_gpio driver? Yes, it is used
>>> by not converted boards.  
>>
>> This is a SPI driver though.
>>
>>>> I recall the SPL jumps back
>>>> to BootROM when loading the U-Boot proper. So if not, drop it.
>>>>
>>>> Also, please don't do partial conversion for iMX28 only, do the
>>>> iMX23 part as well, it cannot be hard.  
>>>
>>> Maybe it is not hard, but I cannot test it properly as I don't have
>>> i.MX23 device. If you are offering your help with testing (i.e. you
>>> do have the access to i.MX23 device and you will test those
>>> changes) I can add support for it.
>>>
>>> Otherwise, NO, I will not add ANY new untested code.  
>>
>> In general, you don't have to add any code, the iMX23/28 SPI IP is
>> very much the same hardware, DTTO for most of the other blocks. If
>> there are any differences between iMX23/28, they are already handled
>> in the existing driver(s).
>>
>> Half-way converted drivers in fact increase maintenance burden,
>> because then we have to deal with two different variants of the code,
>> instead of only one.
> 
> I cannot agree with this sentence.

Do you think maintaining - one DM driver which supports both iMX23 and
iMX28 - is more burden than maintaining - one driver which supports DM,
but only for iMX28 and non-DM for iMX23 and iMX28 ? I don't think so.

> The conversion would be done for
> i.MX28, which is then tested and validated (and clearly stated in the
> cover letter/commit message that only supported was i.MX28).>
> If I don't need to adjust common, reused code (which already supports
> both variants as it is the case with mxs_spi.c), then I don't mind.

Well, that is what I said above, you don't.

> For more intrusive changes - the driver needs to be tested and
> validated (by somebody who has the HW for testing).

That's up to board maintainers.

>> That's why I would like to see this practice go
>> away wherever possible, and in this case it is possible.
> 
> In this particular case it is possible to add support for both as SoC
> specific changes (i.MX23 vs i.MX28) is performed in common code (e.g.
> mxs_spi_xfer_dma).

Both SPI and DMA blocks are basically the same on iMX23 and iMX28.

>> If you need someone to test your changes, CC the board maintainers,
>> that's standard practice.
> 
> As fair as I remember only Angelo and Michael had also interest in
> testing converted code for i.MX28 based board.
> 
> There was NO reply from other people when this (and few others) driver
> was marked as DEPRECATED.

Well, too bad, clearly the interest in this platform is low.
That does not mean we should do sub-par upstream work, does it ?

-- 
Best regards,
Marek Vasut