From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michael Walle Subject: Re: [PATCH 0/6] NXP DSPI bugfixes and support for LS1028A Date: Mon, 09 Mar 2020 19:59:37 +0100 Message-ID: <0b1da58b9d34b6bdb7617f5340d341ac@walle.cc> References: <20200309145624.10026-1-olteanv@gmail.com> <6da04c9a17fa9e6259a462cb52312930@walle.cc> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Cc: Mark Brown , linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, lkml , Shawn Guo , Rob Herring , Mark Rutland , devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Esben Haabendal , angelo-BIYBQhTR83Y@public.gmane.org, andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org, "Gustavo A. R. Silva" , Wei Chen , Mohamed Hosny , peng.ma-3arQi8VN3Tc@public.gmane.org To: Vladimir Oltean Return-path: In-Reply-To: Sender: linux-spi-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-ID: Am 2020-03-09 19:48, schrieb Vladimir Oltean: > On Mon, 9 Mar 2020 at 20:31, Michael Walle wrote: >> >> Am 2020-03-09 19:14, schrieb Vladimir Oltean: >> > On Mon, 9 Mar 2020 at 20:03, Michael Walle wrote: >> >> Am 2020-03-09 15:56, schrieb Vladimir Oltean: >> >> > From: Vladimir Oltean >> >> > >> >> > This series addresses a few issues that were missed during the previous >> >> > series "[PATCH 00/12] TCFQ to XSPI migration for NXP DSPI driver", on >> >> > SoCs other than LS1021A and LS1043A. DMA mode has been completely >> >> > broken >> >> > by that series, and XSPI mode never worked on little-endian >> >> > controllers. >> >> > >> >> > Then it introduces support for the LS1028A chip, whose compatible has >> >> > recently been documented here: >> >> > >> >> > https://lore.kernel.org/linux-devicetree/20200218171418.18297-1-michael-QKn5cuLxLXY@public.gmane.org/ >> >> >> >> If it is not compatible with the LS1021A the second compatible string >> >> should be removed. Depending on the other remark about the endianess, >> >> it might still be compatible, though. >> >> >> > >> > Please feel free to remove it. I wasn't actually planning to add it in >> > the first place, but now it that it's there it doesn't really bother >> > anybody either. >> >> But it won't work if the endianess depends on the compatible string ;) >> > > True. So another reason to not have the endianess (read the CMD/TX register offset) depends on the compatible string. Because then it should work also with the ls1021a version, correct? -michael > >> >> >> >> > The device tree for the LS1028A SoC is extended with DMA channels >> >> > definition, such that even though the default operating mode is XSPI, >> >> > one can simply change DSPI_XSPI_MODE to DSPI_DMA_MODE in the >> >> > devtype_data structure of the driver and use that instead. >> >> >> >> wouldn't it make more sense, to use DMA is the dma node is present >> >> in the device tree? otherwise use XSPI mode? I don't think it is >> >> really handy to change the mode inside the driver. >> >> >> > >> > Let's keep it simple. The driver should configure the hardware in the >> > most efficient and least buggy mode available. Right now that is XSPI. >> > The hardware description (aka the device tree) is a separate topic. If >> > there ever arises any situation where there are corner cases with XSPI >> > mode, it's good to have a fallback in the form of DMA mode, and not >> > have to worry about yet another problem (which is that there are 2 >> > sets of device tree blobs in deployment). >> >> Point taken. But this is not how other drivers behave, which uses the >> DMA if its given in the device node. >> > > Also true. > >> Btw. do other SoCs perform better with DMA? >> > > Not that I know of. > My general rule of thumb for this controller is: if it supports XSPI > then use that, otherwise use DMA. Luckily there is just one controller > that supports none of those, and that would be Coldfire, which uses > the braindead EOQ mode. I don't have the hardware to do testing on > that, but in principle if I did, I would have converted that as well > to the more functional but less efficient TCFQ mode (now removed). > >> -michael >> >> > TL;DR: These DMA channels don't really bother anybody but you never >> > know when they might come in handy. >> > >> >> -michael >> >> >> >> > >> >> > For testing, benchmarking and debugging, the mikroBUS connector on the >> >> > LS1028A-RDB is made available via spidev. >> >> > >> >> > Vladimir Oltean (6): >> >> > spi: spi-fsl-dspi: Don't access reserved fields in SPI_MCR >> >> > spi: spi-fsl-dspi: Fix little endian access to PUSHR CMD and TXDATA >> >> > spi: spi-fsl-dspi: Fix oper_word_size of zero for DMA mode >> >> > spi: spi-fsl-dspi: Add support for LS1028A >> >> > arm64: dts: ls1028a: Specify the DMA channels for the DSPI >> >> > controllers >> >> > arm64: dts: ls1028a-rdb: Add a spidev node for the mikroBUS >> >> > >> >> > .../boot/dts/freescale/fsl-ls1028a-rdb.dts | 14 +++++ >> >> > .../arm64/boot/dts/freescale/fsl-ls1028a.dtsi | 6 +++ >> >> > drivers/spi/spi-fsl-dspi.c | 54 +++++++++++++++---- >> >> > 3 files changed, 64 insertions(+), 10 deletions(-) >> > >> > Thanks, >> > -Vladimir > > -Vladimir