From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Brezillon Date: Wed, 12 Dec 2018 22:25:08 +0100 Subject: [U-Boot] [PATCH 03/16] spi: Add non DM version of SPI_MEM In-Reply-To: References: <20181212173228.12281-1-vigneshr@ti.com> <20181212173228.12281-4-vigneshr@ti.com> <20181212214035.43b825b3@bbrezillon> <20181212220205.4ad00b36@bbrezillon> Message-ID: <20181212222508.5c79d762@bbrezillon> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Wed, 12 Dec 2018 22:07:44 +0100 Jagan Teki wrote: > On Wed 12 Dec, 2018, 10:02 PM Boris Brezillon wrote: > > > On Thu, 13 Dec 2018 02:15:16 +0530 > > Jagan Teki wrote: > > > > > On Thu, Dec 13, 2018 at 2:10 AM Boris Brezillon > > > wrote: > > > > > > > > Hi Jagan, > > > > > > > > On Thu, 13 Dec 2018 01:55:08 +0530 > > > > Jagan Teki wrote: > > > > > > > > > On Wed, Dec 12, 2018 at 11:08 PM Vignesh R > > 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...