[U-Boot] [PATCH 03/16] spi: Add non DM version of SPI_MEM

[Boris Brezillon]

On Wed, 12 Dec 2018 22:07:44 +0100
Jagan Teki <jagan at amarulasolutions.com> wrote:

> On Wed 12 Dec, 2018, 10:02 PM Boris Brezillon <boris.brezillon at bootlin.com
> wrote:
> 
> > On Thu, 13 Dec 2018 02:15:16 +0530
> > Jagan Teki <jagan at amarulasolutions.com> wrote:
> >  
> > > On Thu, Dec 13, 2018 at 2:10 AM Boris Brezillon
> > > <boris.brezillon at bootlin.com> wrote:  
> > > >
> > > > Hi Jagan,
> > > >
> > > > On Thu, 13 Dec 2018 01:55:08 +0530
> > > > Jagan Teki <jagan at amarulasolutions.com> wrote:
> > > >  
> > > > > On Wed, Dec 12, 2018 at 11:08 PM Vignesh R <vigneshr at ti.com>  
> > wrote:  
> > > > > >
> > > > > > Add non DM version of SPI_MEM to support easy migration to new SPI  
> > NOR  
> > > > > > framework. This can be removed once DM_SPI conversion is  
> > complete.  
> > > > >
> > > > > Our intention to use new driver to follow dm, why we need to support
> > > > > non-dm? any usecases?  
> > > >
> > > > Looks like we're having the same discussion over and over. Vignesh is
> > > > dropping spi_flash.c which AFAICT was not depending on DM_SPI, so, if
> > > > we want to keep everyone happy while getting rid of some legacy code,
> > > > that's the only solution. DM conversion is a nice goal, but it's kind
> > > > of orthogonal to what Vignesh is working on. If DM_SPI conversion
> > > > happens before the spi-nor stuff is merged  (which I doubt) then this
> > > > patch can simply be dropped.  
> > >
> > > spi_flash.c is a core code not a specific driver it belongs. spi-mem
> > > is new feature driver how come new driver will support legacy non-dm
> > > do we have legacy use for that(ie what I'm asking about usecase)  
> >
> > I recommend that you read the spi-mem code carefully. spi-mem is not
> > driver specific, it's a thin layer on top of spi and driver *can* (but
> > are not forced to) provide optimized methods to execute spi-mem
> > operations. When that's not the case, the implementation falls back to
> > regular spi transfers. AFAIK, both DM and non-DM drivers support
> > regular spi transfers, right? So why should we depend on DM_SPI? And
> > more importantly, if we do that, that means we can't get rid of
> > spi_flash.c since some users might still have non-DM SPI drivers, which
> > in turn means we keep more legacy code for no good reasons.
> >  
> 
> I understand spi-mem is core file, but new code too.

Sorry, I don't get it.

> 
> 
> > You want non-DM SPI controller drivers to go away, then remove them,
> > instead of blocking other changes using this excuse.
> >  
> 
> Please understand uboot development flow, legacy driver can be removed if
> possible once migration expire and NEW drivers or code must be dm driven.

Sorry, but I think you're the one misunderstanding what we are trying
to do here. Vignesh changes have simply no impact on the DM SPI
conversion you're aiming at. All Vignesh does is provide a dummy
wrapper for non-DM drivers, which would probably have been implemented
by Miquel if you had not been so insistent on your precious DM_SPI
conversion. That was not really a problem for spi-nand, as we were
adding support for a new feature. This is not the case here. SPI NORs
are already partially supported by the u-boot spi flash layer, and we
need to keep things in a working state for those that were using it and
didn't have their SPI controller drivers converted to the DM. This
leaves us 2 options:

1/ keep the sf_flash code as is and add a new spi-nor code base 
2/ replace spi_flash code by the spi-nor layer imported from Linux

Vignesh chose option #2 which has the benefit of avoiding code
duplication. Given the discussion we're having right now, I'm wondering
if it wouldn't be easier to go for option #1 in order to avoid those
endless discussions...