From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nobuhiro Iwamatsu Subject: Re: [PATCH] spi: sh-msiof: Fix limit maximum word transfer size of FIFO size Date: Wed, 18 Mar 2015 12:50:31 +0900 Message-ID: <5508F607.7010104@renesas.com> References: <1426127476-18456-1-git-send-email-nobuhiro.iwamatsu.yj@renesas.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit Cc: Mark Brown , Linux-sh list , linux-spi , Yoshihiro Shimoda To: Geert Uytterhoeven Return-path: In-Reply-To: Sender: linux-sh-owner@vger.kernel.org List-Id: linux-spi.vger.kernel.org Hi, (2015/03/16 16:50), Geert Uytterhoeven wrote: > Hi Iwamatsu-san, > > On Mon, Mar 16, 2015 at 2:18 AM, Nobuhiro Iwamatsu > wrote: >> 2015-03-12 21:35 GMT+09:00 Geert Uytterhoeven: >>> On Thu, Mar 12, 2015 at 3:31 AM, Nobuhiro Iwamatsu >>> wrote: > >>>> - /* limit maximum word transfer to rx/tx fifo size */ >>>> - if (tx_buf) >>>> - words = min_t(int, words, p->tx_fifo_size); >>>> - if (rx_buf) >>>> - words = min_t(int, words, p->rx_fifo_size); > >>> Sorry, I fail to see what exactly this is fixing. >>> >>> If SPI_MASTER_MUST_TX is set, all hardware SPI transfers are either >>> a) transmit-only. >>> b) bidirectional (transmit buffer may be a dummy, provided by the SPI core). >>> >>> For case a, only the TX FIFO size matters. >>> - The original code ignored the RX FIFO size (rx_buf == NULL), >>> - After your change, it always uses the minimum of the TX and RX FIFO sizes >>> (granted, the RX FIFO size is larger, so this doesn't make a difference on >>> current hardware). >> >> Yes. >> >>> >>> For case b, both FIFO sizes matter, and the original code handled that fine >>> (tx_buf != NULL, rx_buf != NULL). >>> >>> What am I missing? >>> Are you using a backport with broken SPI_MASTER_MUST_TX handling in the SPI >>> core? >> >> When tx_buf != NULL and rx_buf != NULL, current code uses FIFO size of >> rx_buffer. >> Since TX FIFO size is smaller than RX FIFO size, corrent code set the >> wrong value to SITMDR2 register. > > if tx_buf != NULL and rx_buf != NULL, current code does: > > words = min_t(int, words, p->tx_fifo_size); > words = min_t(int, words, p->rx_fifo_size); > > Hence words will be the minimum of the original value of words, tx_fifo_size, > and rx_fifo_size. What's wrong about that? > I see. I understood about this. Sorry, my bad. Mark, if you do not apply this patch to your repository, could you ignore this? >> Therefore, this patch selects a smaller FIFO size, adds the function to set. >> >> Does this has become a description? >> >>> >>> I've just verified that with today's tree (renesas-drivers-2015-03-12-v4.0-rc3), >>> SPI_MASTER_MUST_TX works fine, and a dummy tx_buf is passed when needed. >> >> I think correctly work on hardware. However, driver has been set to >> the correct register value? > > I printed the value of words, which is passed to sh_msiof_spi_set_mode_regs(). > > Gr{oetje,eeting}s, > > Geert > Best regards, Nobuhiro