From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Brezillon Date: Fri, 25 Jan 2019 19:00:31 +0100 Subject: [U-Boot] [PATCH 2/3] spi: Add support for the Aspeed ast2500 SPI controllers In-Reply-To: References: <20180910141655.20944-1-clg@kaod.org> <20180910141655.20944-3-clg@kaod.org> <09c407b3-be92-6ded-ac87-1e8b79a59761@kaod.org> <337686c0-cad4-1dbb-ece1-f1bf578838f6@kaod.org> <20181010093255.17130d10@bbrezillon> <5262d43d-ba51-fcbc-eb97-64e373de4cd9@kaod.org> Message-ID: <20190125190002.3754813c@bbrezillon> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: u-boot@lists.denx.de +Vignesh Hi Cédric, On Fri, 25 Jan 2019 18:28:02 +0100 Cédric Le Goater wrote: > Hello > > On 10/10/18 2:02 PM, Cédric Le Goater wrote: > > Hello Boris, > > > > On 10/10/18 9:32 AM, Boris Brezillon wrote: > >> Hi Cédric, > >> > >> On Wed, 10 Oct 2018 11:46:56 +0530 > >> Jagan Teki wrote: > >> > >>> On Mon, Oct 8, 2018 at 11:32 AM Cédric Le Goater wrote: > >>>> > >>>> On 10/4/18 5:57 PM, Jagan Teki wrote: > >>>>> On Fri, Sep 28, 2018 at 5:20 PM Cédric Le Goater wrote: > >>>>>> > >>>>>> Hello Simon, > >>>>>> > >>>>>> > >>>>>> The Aspeed AST2500 FMC controller can handle SPI flash and NOR flash memory, > >>>>>> and the Aspeed AST2500 SPI Flash Controllers only SPI. If there is some > >>>>>> misunderstanding on this driver, it might come from the fact it is closer > >>>>>> to a SPI-NOR driver like we have in Linux, than a generic SPI driver. > >>>>>> The stm32 SPI driver is somewhat similar. > >>>>>> > >>>>>> Should we move it under drivers/mtd/spi/ maybe ? > >>>>> > >>>>> Seems with new spi-mem in Linux flash memory driver rely on spi-mem > >>>>> instead of mtd/spi-nor. So I think you can handle this via new > >>>>> spi-mem. have you send any patches to Linux? > >>>> > >>>> No, not yet. The patchset is sent : > >>>> > >>>> https://patchwork.ozlabs.org/cover/933293/ > >>>> > >>>> is not using spimem. I was not aware of that change in the spi-nor layer > >>>> at the time. I will take a look. > >> > >> Indeed, if you have some time to convert the Linux aspeed driver to > >> the spi-mem interface that would be appreciated. > > > > Yes. That's the plan. I have a series on the way but I will see if I can > > rework a v2 to use spi-mem. > > I took a look at spi-mem and worked on an initial spi-aspeed-smc driver > using the new framework in Linux 5.0.x. It looks good but I have a couple > of questions. Great! > > > The first is about performing direct accesses on the AHB window on which > the flash contents is mapped. We have introduced the dirmap API/interface exactly for this purpose, and the SPI NOR layer will use it in 5.1 (see [1]). > > How do you distinguish a flash read (fast, dual, etc) from a RDSFPD command > for instance ? Why do you need to know the access type? > Are the drivers expected to check the SPI OP command and > depending on the target/command redirect to the appropriate address space ? Definitely not, the SPI MEM layer is supposed to be memory-type agnostic, so you should not guess the operation type based on the opcode. For direct mapping accesses, just implement the ->dirmap_xxx hooks at the controller level and you'll be able to use the feature. > > Also, Aspeed SPI controllers have a Read Timing Compensation Register which > defines different data input delay cycles depending on SPI clock rates. This > register is supposed to be tuned when the flash chip characteristics are > known, after the first bus scan. Is there a way to know that our SPI slave > is alive and well detected before starting hammering successive reads on it > to see how it behaves. Vignesh mentioned that a while back (couldn't find the thread where this discussion happened) and I suggested adding a new hook to do this "link training" process where you'd pass a spi_mem_op template + the expected result so that the controller can test different setups until it finds a working one. > > > I think the U-Boot and Linux driver will be very similar wrt the issues > above ? I hope so. Thanks for working on this conversion. Boris [1]https://patchwork.kernel.org/patch/10670755/