From mboxrd@z Thu Jan 1 00:00:00 1970 From: Miquel Raynal Date: Thu, 13 Dec 2018 10:41:28 +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> <20181212222508.5c79d762@bbrezillon> Message-ID: <20181213104128.2a4e0414@xps13> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable To: u-boot@lists.denx.de Hi Vignesh, Vignesh R wrote on Thu, 13 Dec 2018 15:08:32 +0530: > On 13/12/18 2:55 AM, Boris Brezillon wrote: > > On Wed, 12 Dec 2018 22:07:44 +0100 > > Jagan Teki wrote: > > =20 > >> On Wed 12 Dec, 2018, 10:02 PM Boris Brezillon >> wrote: > >> =20 > >>> On Thu, 13 Dec 2018 02:15:16 +0530 > >>> Jagan Teki wrote: > >>> =20 > >>>> On Thu, Dec 13, 2018 at 2:10 AM Boris Brezillon > >>>> wrote: =20 > >>>>> > >>>>> Hi Jagan, > >>>>> > >>>>> On Thu, 13 Dec 2018 01:55:08 +0530 > >>>>> Jagan Teki wrote: > >>>>> =20 > >>>>>> On Wed, Dec 12, 2018 at 11:08 PM Vignesh R =20 > >>> wrote: =20 > >>>>>>> > >>>>>>> Add non DM version of SPI_MEM to support easy migration to new SP= I =20 > >>> NOR =20 > >>>>>>> framework. This can be removed once DM_SPI conversion is =20 > >>> complete. =20 > >>>>>> > >>>>>> Our intention to use new driver to follow dm, why we need to suppo= rt > >>>>>> non-dm? any usecases? =20 > >>>>> > >>>>> 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 cod= e, > >>>>> that's the only solution. DM conversion is a nice goal, but it's ki= nd > >>>>> of orthogonal to what Vignesh is working on. If DM_SPI conversion > >>>>> happens before the spi-nor stuff is merged (which I doubt) then th= is > >>>>> patch can simply be dropped. =20 > >>>> > >>>> 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, whi= ch > >>> in turn means we keep more legacy code for no good reasons. > >>> =20 > >> > >> I understand spi-mem is core file, but new code too. =20 > >=20 > > Sorry, I don't get it. > > =20 > >> > >> =20 > >>> You want non-DM SPI controller drivers to go away, then remove them, > >>> instead of blocking other changes using this excuse. > >>> =20 > >> > >> Please understand uboot development flow, legacy driver can be removed= if > >> possible once migration expire and NEW drivers or code must be dm driv= en. =20 > >=20 > > 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: > >=20 > > 1/ keep the sf_flash code as is and add a new spi-nor code base=20 > > 2/ replace spi_flash code by the spi-nor layer imported from Linux > >=20 > > 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... > > =20 >=20 > Boris, thanks for chiming in! This is exactly what I had in mind. >=20 > To add, I did start with #1 by simply adding support for 4 byte > addressing. But, then released I need spi-mem to communicate this > protocol info to SPI drivers, then found Quad Enable detection logic to > be incomplete and so on.. Finally released I would end up with code > exactly similar to Linux SPI-NOR(with addition of SFDP logic). Therefore > switched to #2 ;) >=20 >=20 I also share (strongly) the same point of view: #2 is the best way IMHO.=20 Thanks, Miqu=C3=A8l