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

[Lukasz Majewski]

Hi Marek,

> On 6/17/19 3:41 PM, Lukasz Majewski wrote:
> > On Mon, 17 Jun 2019 15:23:55 +0200
> > Marek Vasut <marex at denx.de> wrote:
> >   
> >> 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.  
> > 
> > To make myself clear - If I can reuse the common code (which
> > supports both imx23 and 28) for DM/DTS conversion then I'm OK with
> > doing so.
> > 
> > If you require me to add untested code specific to i.MX23 - then
> > NO.  
> 
> Yes, you can.

If possible, by reusing the old, common working code, I can add this to
the converted driver.

But I will not any new untested code.

> 
> >>  
> >>> 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 I can reuse the common code, then I'm fine to do it.
> >   
> >>  
> >>>> 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.  
> > 
> > This means that people are using either some old U-Boot version, or
> > there are a few people who want to refurbish the old HW with new
> > code (e.g. Michael, Angelo).  
> 
> Yes
> 
> >> That does not mean we should do sub-par upstream work, does it ?
> >>  
> > 
> > We shall not add untested code.  
> 
> You cannot test every single platform in existence. Post patches and
> let board maintainers test them ; if they won't, too bad, their
> platform might become broken. There's no other way to move forward
> without dragging behind a tremendous amount of ancient baggage, and
> that in turn is not viable.
> 




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190617/08f590ec/attachment.sig>