public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Lukasz Majewski <lukma@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v3 5/5] ARM: dm: spi: Add support DM/DTS for i.MX28 mxs SPI driver (DM_SPI conversion)
Date: Mon, 17 Jun 2019 15:41:29 +0200	[thread overview]
Message-ID: <20190617154129.39c29b67@jawa> (raw)
In-Reply-To: <000b54e7-7a4f-9c4c-d362-201f93d0e45f@denx.de>

On Mon, 17 Jun 2019 15:23:55 +0200
Marek Vasut <marex@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@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.

> 
> > 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).

> That does not mean we should do sub-par upstream work, does it ?
> 

We shall not add untested code.



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/95dbcd99/attachment.sig>

  reply	other threads:[~2019-06-17 13:41 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-15 22:34 [U-Boot] [PATCH v3 0/5] DM: Convert i.MX28 gpio, pinmux, spi and eth drivers to DM/DTS Lukasz Majewski
2019-06-15 22:34 ` [U-Boot] [PATCH v3 1/5] ARM: dts: imx: Copy imx28 device tree related files from Linux kernel (v5.1.9) Lukasz Majewski
2019-06-15 22:43   ` Marek Vasut
2019-06-17  6:57     ` Lukasz Majewski
2019-06-17 10:13       ` Marek Vasut
2019-06-15 22:34 ` [U-Boot] [PATCH v3 2/5] ARM: imx: net: Enable support for i.MX28 DM_ETH in the fec_mxc.c driver Lukasz Majewski
2019-06-15 22:43   ` Marek Vasut
2019-06-17  6:57     ` Lukasz Majewski
2019-06-15 22:34 ` [U-Boot] [PATCH v3 3/5] ARM: dm: mxs: gpio: Add support for DM/DTS in the mxs_gpio.c driver (DM_GPIO) Lukasz Majewski
2019-06-15 22:53   ` Marek Vasut
2019-06-17  8:37     ` Lukasz Majewski
2019-06-17 10:20       ` Marek Vasut
2019-06-17 13:01         ` Lukasz Majewski
2019-06-17 13:34           ` Marek Vasut
2019-06-18  6:57             ` Lukasz Majewski
2019-06-18 10:33               ` Marek Vasut
2019-06-18 10:56                 ` Lukasz Majewski
2019-06-18 11:04                   ` Marek Vasut
2019-06-15 22:34 ` [U-Boot] [PATCH v3 4/5] ARM: imx: pinctrl: Add support for i.MX28 mxs pinctrl driver Lukasz Majewski
2019-06-15 23:00   ` Marek Vasut
2019-06-17  7:43     ` Lukasz Majewski
2019-06-17 10:23       ` Marek Vasut
2019-06-15 22:34 ` [U-Boot] [PATCH v3 5/5] ARM: dm: spi: Add support DM/DTS for i.MX28 mxs SPI driver (DM_SPI conversion) Lukasz Majewski
2019-06-15 23:02   ` Marek Vasut
2019-06-17  6:49     ` Lukasz Majewski
2019-06-17 10:06       ` Marek Vasut
2019-06-17 12:27         ` Lukasz Majewski
2019-06-17 13:23           ` Marek Vasut
2019-06-17 13:41             ` Lukasz Majewski [this message]
2019-06-17 13:46               ` Marek Vasut
2019-06-17 14:57                 ` Lukasz Majewski
2019-06-17 15:00                   ` Marek Vasut

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190617154129.39c29b67@jawa \
    --to=lukma@denx.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox