From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 7E013C43334 for ; Mon, 18 Jul 2022 10:21:39 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id A984D8402E; Mon, 18 Jul 2022 12:21:37 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Received: by phobos.denx.de (Postfix, from userid 109) id 8E6678402F; Mon, 18 Jul 2022 12:21:36 +0200 (CEST) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by phobos.denx.de (Postfix) with ESMTP id 9321D8402C for ; Mon, 18 Jul 2022 12:21:32 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=andre.przywara@arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 3D5AE1042; Mon, 18 Jul 2022 03:21:32 -0700 (PDT) Received: from donnerap.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id A0A563F70D; Mon, 18 Jul 2022 03:21:30 -0700 (PDT) Date: Mon, 18 Jul 2022 11:21:28 +0100 From: Andre Przywara To: qianfan Cc: Jagan Teki , u-boot@lists.denx.de, Chen-Yu Tsai , Maxime Ripard , Samuel Holland , Simon Glass , Jesse Taube Subject: Re: [PATCH v1] drivers: spi: sunxi: Fix spi speed settting Message-ID: <20220718112128.0ef71a06@donnerap.cambridge.arm.com> In-Reply-To: <57e9b199-5725-fc7a-c5a2-2acaa774d20f@163.com> References: <20220609090939.25828-1-qianfanguijin@163.com> <20220628013451.4d452a16@slackpad.lan> <971f87b7-772f-e757-3e9b-cafc413dbd40@163.com> <4bd00f93-55b9-563c-4d0f-27041ff972ab@163.com> <57e9b199-5725-fc7a-c5a2-2acaa774d20f@163.com> Organization: ARM X-Mailer: Claws Mail 3.18.0 (GTK+ 2.24.32; aarch64-unknown-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.6 at phobos.denx.de X-Virus-Status: Clean On Sat, 2 Jul 2022 16:20:37 +0800 qianfan wrote: Hi Qianfan, again our mailserver dropped this email, so sorry for the delay! > =E5=9C=A8 2022/7/2 15:50, qianfan =E5=86=99=E9=81=93: > > > > > > =E5=9C=A8 2022/7/2 11:09, qianfan =E5=86=99=E9=81=93: =20 > >> > >> > >> =E5=9C=A8 2022/6/28 8:34, Andre Przywara =E5=86=99=E9=81=93: =20 > >>> On Thu,=C2=A0 9 Jun 2022 17:09:39 +0800 > >>> qianfanguijin@163.com wrote: > >>> > >>> Hi Qianfan, > >>> =20 > >>>> From: qianfan Zhao > >>>> > >>>> dm_spi_claim_bus run spi_set_speed_mode first and then ops->claim_bu= s, > >>>> but spi clock is enabled when sun4i_spi_claim_bus, that will make > >>>> sun4i_spi_set_speed doesn't work. =20 > >>> Thanks for bringing this up, and sorry for the delay (please CC: the > >>> U-Boot sunxi maintainers!). > >>> So this is very similar to the patch as I sent earlier: > >>> https://lore.kernel.org/u-boot/20220503212040.27884-3-andre.przywara@= arm.com/=20 > >>> > >>> > >>> Can you please check whether this works for you as well, then reply to > >>> that patch? > >>> I put my version of the patch plus more fixes and F1C100s support to: > >>> https://source.denx.de/u-boot/custodians/u-boot-sunxi/-/commits/next/ > >>> > >>> Also I am curious under what circumstances and on what board you saw= =20 > >>> the > >>> issue? In my case it was on the F1C100s, which has a higher base clock > >>> (200 MHz instead of 24 MHz), so everything gets badly overclocked. =20 > >> I tested based on those two commits: > >> > >> spi: sunxi: refactor SPI speed/mode programming > >> spi: sunxi: improve SPI clock calculation > >> > >> And there are a couple of questions: > >> > >> 1. sun4i_spi_of_to_plat try reading "spi-max-frequency" from the spi=20 > >> bus node: > >> > >> static int sun4i_spi_of_to_plat(struct udevice *bus) > >> { > >> =C2=A0=C2=A0 =C2=A0struct sun4i_spi_plat *plat =3D dev_get_plat(bus); > >> =C2=A0=C2=A0 =C2=A0int node =3D dev_of_offset(bus); > >> > >> =C2=A0=C2=A0 =C2=A0plat->base =3D dev_read_addr(bus); > >> =C2=A0=C2=A0 =C2=A0plat->variant =3D (struct sun4i_spi_variant=20 > >> *)dev_get_driver_data(bus); > >> =C2=A0=C2=A0 =C2=A0plat->max_hz =3D fdtdec_get_int(gd->fdt_blob, node, > >> =C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0= =C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 "spi-max-frequency", > >> =C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0= =C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 SUN4I_SPI_DEFAULT_RATE); > >> > >> =C2=A0=C2=A0 =C2=A0if (plat->max_hz > SUN4I_SPI_MAX_RATE) > >> =C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0 plat->max_hz =3D SUN4I_SPI_MAX_R= ATE; > >> > >> =C2=A0=C2=A0 =C2=A0return 0; > >> } > >> > >> Seems this is not a correct way. "spi-max-frequency" should reading=20 > >> from spi device, > >> not spi bus. On my dts, no "spi-max-frequency" prop on spi bus node,=20 > >> this will make > >> plat->max_hz has default SUN4I_SPI_DEFAULT_RATE(1M) value. > >> > >> &spi2 { > >> =C2=A0=C2=A0 =C2=A0pinctrl-names =3D "default"; > >> =C2=A0=C2=A0 =C2=A0pinctrl-0 =3D <&spi2_cs0_pb_pin &spi2_pb_pins>; > >> =C2=A0=C2=A0 =C2=A0status =3D "okay"; > >> > >> =C2=A0=C2=A0 =C2=A0lcd@0 { > >> =C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0 compatible =3D "sitronix,st75161= "; > >> =C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0 spi-max-frequency =3D <12000000>; > >> =C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0 reg =3D <0>; > >> =C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0 spi-cpol; > >> =C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0 spi-cpha; > >> > >> So on my patch, I had changed the default plat->max_hz to=20 > >> SUN4I_SPI_MAX_RATE. > >> > >> 2. When I changed the default plat->max_hz to SUN4I_SPI_MAX_RATE: > >> > >> 2.1: sun4i_spi_set_speed_mode doesn't consider when div =3D 1(freq =3D= =20 > >> SUNXI_INPUT_CLOCK), > >> the spi running in 12M even if the spi-max-frequency is setted to 24M. > >> > >> 2.2: on my R40 based board, spi can't work when the spi clock <=3D 6M. > >> I had check the CCR register, the value is correct, from logic analyzer > >> only the first byte is sent. Next is the serial console logs: > >> > >> spi clock =3D 6M: > >> CCR: 00001001 > >> ERROR: sun4i_spi: Timeout transferring data > >> ERROR: sun4i_spi: Timeout transferring data > >> ERROR: sun4i_spi: Timeout transferring data > >> ... > >> > >> spi clock =3D 4M: > >> CCR: 00001002 > >> ERROR: sun4i_spi: Timeout transferring data > >> ERROR: sun4i_spi: Timeout transferring data > >> ERROR: sun4i_spi: Timeout transferring data > >> ERROR: sun4i_spi: Timeout transferring data > >> ERROR: sun4i_spi: Timeout transferring data > >> ... =20 > > Add udelay(1) before sun4i_spi_drain_fifo in sun4i_spi_xfer can fix it. > > But I don't know why. =20 > >> =20 > >>> > >>> Thanks! > >>> Andre > >>> =20 > >>>> Fix it. > >>>> > >>>> Signed-off-by: qianfan Zhao > >>>> --- > >>>> =C2=A0 drivers/spi/spi-sunxi.c | 78=20 > >>>> ++++++++++++++++------------------------- > >>>> =C2=A0 1 file changed, 30 insertions(+), 48 deletions(-) > >>>> > >>>> diff --git a/drivers/spi/spi-sunxi.c b/drivers/spi/spi-sunxi.c > >>>> index b6cd7ddafa..1043cde976 100644 > >>>> --- a/drivers/spi/spi-sunxi.c > >>>> +++ b/drivers/spi/spi-sunxi.c > >>>> @@ -224,6 +224,7 @@ err_ahb: > >>>> =C2=A0 static int sun4i_spi_claim_bus(struct udevice *dev) > >>>> =C2=A0 { > >>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 struct sun4i_spi_priv *priv =3D dev_g= et_priv(dev->parent); > >>>> +=C2=A0=C2=A0=C2=A0 u32 div, reg; > >>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 int ret; > >>>> =C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ret =3D sun4i_spi_set_clock(de= v->parent, true); > >>>> @@ -233,12 +234,38 @@ static int sun4i_spi_claim_bus(struct udevice= =20 > >>>> *dev) > >>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 setbits_le32(SPI_REG(priv, SPI_GCR), = SUN4I_CTL_ENABLE | > >>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0 SUN4I_CTL_MASTER | SPI_BIT(priv, SPI_GCR_TP)); > >>>> =C2=A0 +=C2=A0=C2=A0=C2=A0 /* Setup clock divider */ > >>>> +=C2=A0=C2=A0=C2=A0 div =3D SUN4I_SPI_MAX_RATE / (2 * priv->freq); > >>>> +=C2=A0=C2=A0=C2=A0 reg =3D readl(SPI_REG(priv, SPI_CCR)); > >>>> + > >>>> +=C2=A0=C2=A0=C2=A0 if (div <=3D (SUN4I_CLK_CTL_CDR2_MASK + 1)) { > >>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (div > 0) > >>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 = div--; > >>>> + > >>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 reg &=3D ~(SUN4I_CLK_CTL= _CDR2_MASK | SUN4I_CLK_CTL_DRS); > >>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 reg |=3D SUN4I_CLK_CTL_C= DR2(div) | SUN4I_CLK_CTL_DRS; > >>>> +=C2=A0=C2=A0=C2=A0 } else { > >>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 div =3D __ilog2(SUN4I_SP= I_MAX_RATE) - __ilog2(priv->freq); > >>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 reg &=3D ~((SUN4I_CLK_CT= L_CDR1_MASK << 8) | SUN4I_CLK_CTL_DRS); > >>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 reg |=3D SUN4I_CLK_CTL_C= DR1(div); > >>>> +=C2=A0=C2=A0=C2=A0 } > >>>> + > >>>> +=C2=A0=C2=A0=C2=A0 writel(reg, SPI_REG(priv, SPI_CCR)); > >>>> + > >>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (priv->variant->has_soft_reset) > >>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 setbits_le32(= SPI_REG(priv, SPI_GCR), > >>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 SPI_BIT(priv, SPI_GCR_SRST)); > >>>> =C2=A0 -=C2=A0=C2=A0=C2=A0 setbits_le32(SPI_REG(priv, SPI_TCR), SPI_= BIT(priv,=20 > >>>> SPI_TCR_CS_MANUAL) | > >>>> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0 SPI_BIT(priv, SPI_TCR_CS_ACTIVE_LOW)); > >>>> +=C2=A0=C2=A0=C2=A0 /* Setup the transfer control register */ > >>>> +=C2=A0=C2=A0=C2=A0 reg =3D SPI_BIT(priv, SPI_TCR_CS_MANUAL) | > >>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 SPI_BIT(priv= , SPI_TCR_CS_ACTIVE_LOW); > >>>> + > >>>> +=C2=A0=C2=A0=C2=A0 if (priv->mode & SPI_CPOL) > >>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 reg |=3D SPI_BIT(priv, S= PI_TCR_CPOL); > >>>> +=C2=A0=C2=A0=C2=A0 if (priv->mode & SPI_CPHA) > >>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 reg |=3D SPI_BIT(priv, S= PI_TCR_CPHA); > >>>> + > >>>> +=C2=A0=C2=A0=C2=A0 writel(reg, SPI_REG(priv, SPI_TCR)); > >>>> =C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 return 0; > >>>> =C2=A0 } > >>>> @@ -329,67 +356,22 @@ static int sun4i_spi_set_speed(struct udevice= =20 > >>>> *dev, uint speed) > >>>> =C2=A0 { > >>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 struct sun4i_spi_plat *plat =3D dev_g= et_plat(dev); > >>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 struct sun4i_spi_priv *priv =3D dev_g= et_priv(dev); > >>>> -=C2=A0=C2=A0=C2=A0 unsigned int div; > >>>> -=C2=A0=C2=A0=C2=A0 u32 reg; > >>>> =C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (speed > plat->max_hz) > >>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 speed =3D pla= t->max_hz; > >>>> =C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (speed < SUN4I_SPI_MIN_RATE) > >>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 speed =3D SUN= 4I_SPI_MIN_RATE; > >>>> -=C2=A0=C2=A0=C2=A0 /* > >>>> -=C2=A0=C2=A0=C2=A0=C2=A0 * Setup clock divider. > >>>> -=C2=A0=C2=A0=C2=A0=C2=A0 * > >>>> -=C2=A0=C2=A0=C2=A0=C2=A0 * We have two choices there. Either we can= use the clock > >>>> -=C2=A0=C2=A0=C2=A0=C2=A0 * divide rate 1, which is calculated thank= s to this formula: > >>>> -=C2=A0=C2=A0=C2=A0=C2=A0 * SPI_CLK =3D MOD_CLK / (2 ^ (cdr + 1)) > >>>> -=C2=A0=C2=A0=C2=A0=C2=A0 * Or we can use CDR2, which is calculated = with the formula: > >>>> -=C2=A0=C2=A0=C2=A0=C2=A0 * SPI_CLK =3D MOD_CLK / (2 * (cdr + 1)) > >>>> -=C2=A0=C2=A0=C2=A0=C2=A0 * Whether we use the former or the latter = is set through the > >>>> -=C2=A0=C2=A0=C2=A0=C2=A0 * DRS bit. > >>>> -=C2=A0=C2=A0=C2=A0=C2=A0 * > >>>> -=C2=A0=C2=A0=C2=A0=C2=A0 * First try CDR2, and if we can't reach th= e expected > >>>> -=C2=A0=C2=A0=C2=A0=C2=A0 * frequency, fall back to CDR1. > >>>> -=C2=A0=C2=A0=C2=A0=C2=A0 */ > >>>> - > >>>> -=C2=A0=C2=A0=C2=A0 div =3D SUN4I_SPI_MAX_RATE / (2 * speed); > >>>> -=C2=A0=C2=A0=C2=A0 reg =3D readl(SPI_REG(priv, SPI_CCR)); > >>>> - > >>>> -=C2=A0=C2=A0=C2=A0 if (div <=3D (SUN4I_CLK_CTL_CDR2_MASK + 1)) { > >>>> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (div > 0) > >>>> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 = div--; > >>>> - > >>>> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 reg &=3D ~(SUN4I_CLK_CTL= _CDR2_MASK | SUN4I_CLK_CTL_DRS); > >>>> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 reg |=3D SUN4I_CLK_CTL_C= DR2(div) | SUN4I_CLK_CTL_DRS; > >>>> -=C2=A0=C2=A0=C2=A0 } else { > >>>> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 div =3D __ilog2(SUN4I_SP= I_MAX_RATE) - __ilog2(speed); > >>>> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 reg &=3D ~((SUN4I_CLK_CT= L_CDR1_MASK << 8) | SUN4I_CLK_CTL_DRS); > >>>> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 reg |=3D SUN4I_CLK_CTL_C= DR1(div); > >>>> -=C2=A0=C2=A0=C2=A0 } > >>>> =C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 priv->freq =3D speed; > >>>> -=C2=A0=C2=A0=C2=A0 writel(reg, SPI_REG(priv, SPI_CCR)); > >>>> - > >>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 return 0; > >>>> =C2=A0 } > >>>> =C2=A0 =C2=A0 static int sun4i_spi_set_mode(struct udevice *dev, uin= t mode) > >>>> =C2=A0 { > >>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 struct sun4i_spi_priv *priv =3D dev_g= et_priv(dev); > >>>> -=C2=A0=C2=A0=C2=A0 u32 reg; > >>>> - > >>>> -=C2=A0=C2=A0=C2=A0 reg =3D readl(SPI_REG(priv, SPI_TCR)); > >>>> -=C2=A0=C2=A0=C2=A0 reg &=3D ~(SPI_BIT(priv, SPI_TCR_CPOL) | SPI_BIT= (priv,=20 > >>>> SPI_TCR_CPHA)); > >>>> - > >>>> -=C2=A0=C2=A0=C2=A0 if (mode & SPI_CPOL) > >>>> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 reg |=3D SPI_BIT(priv, S= PI_TCR_CPOL); > >>>> - > >>>> -=C2=A0=C2=A0=C2=A0 if (mode & SPI_CPHA) > >>>> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 reg |=3D SPI_BIT(priv, S= PI_TCR_CPHA); > >>>> =C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 priv->mode =3D mode; > >>>> -=C2=A0=C2=A0=C2=A0 writel(reg, SPI_REG(priv, SPI_TCR)); > >>>> - > >>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 return 0; > >>>> =C2=A0 } > >>>> =C2=A0 @@ -441,7 +423,7 @@ static int sun4i_spi_of_to_plat(struct=20 > >>>> udevice *bus) > >>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 plat->variant =3D (struct sun4i_spi_v= ariant=20 > >>>> *)dev_get_driver_data(bus); > >>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 plat->max_hz =3D fdtdec_get_int(gd->f= dt_blob, node, > >>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 "s= pi-max-frequency", > >>>> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 SUN4I_SPI_DEFA= ULT_RATE); > >>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 SUN4I_SPI_MAX_= RATE); > >>>> =C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (plat->max_hz > SUN4I_SPI_M= AX_RATE) > >>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 plat->max_hz = =3D SUN4I_SPI_MAX_RATE; =20 > >> =20 > > =20 > Hi everyone: >=20 > I had fixed "Timeout transferring data" issue and tested on sun8i r40=20 > platform. Yes, Icenowy figured that and posted a very similar patch: https://patchwork.ozlabs.org/project/uboot/patch/20220628064924.390103-1-uw= u@icenowy.me/ I will take her patch, before my series, to make sure we don't introduce non-working commits. > But I don't have a SUN4I_A10 board, could you please test and check this= =20 > patch? I didn't test on an A10, but on H6 and A64, where there is the exact same issue. It it still very odd why this happens, exactly: the old code seems to genuinely wait to 1 second, so plenty of time to send anything out. And if I read the FSR register after the XCH poll returned, I see it being fine, so the previous check should have matched as well. Also I can confirm your other observation: introducing some odd delay *after* the check seems to fix it. So I would very much like to find the real reason for this, but we should fix the existing real-world problems first. If anyone could investigate this further, I would be very grateful. Thanks, Andre > From 514e9396509593515b7fa848cbc4b8eccf948547 Mon Sep 17 00:00:00 2001 > From: qianfan Zhao > Date: Sat, 2 Jul 2022 16:07:18 +0800 > Subject: [PATCH] spi: sunxi: Fix transfer timeout when running at a low > =C2=A0frequency >=20 > sun4i_spi_xfer will report error messages when running at a low > frequency such as 6MHz, at least on SUN8I R40 platform: > ERROR: sun4i_spi: Timeout transferring data >=20 > Fix the waiting condition. >=20 > Signed-off-by: qianfan Zhao > --- > =C2=A0drivers/spi/spi-sunxi.c | 12 +++++------- > =C2=A01 file changed, 5 insertions(+), 7 deletions(-) >=20 > diff --git a/drivers/spi/spi-sunxi.c b/drivers/spi/spi-sunxi.c > index d123adc68a..55b2de8339 100644 > --- a/drivers/spi/spi-sunxi.c > +++ b/drivers/spi/spi-sunxi.c > @@ -400,7 +400,7 @@ static int sun4i_spi_xfer(struct udevice *dev,=20 > unsigned int bitlen, > =C2=A0=C2=A0=C2=A0=C2=A0 struct dm_spi_slave_plat *slave_plat =3D dev_ge= t_parent_plat(dev); >=20 > =C2=A0=C2=A0=C2=A0=C2=A0 u32 len =3D bitlen / 8; > -=C2=A0=C2=A0=C2=A0 u32 rx_fifocnt; > +=C2=A0=C2=A0=C2=A0 u32 tcr; > =C2=A0=C2=A0=C2=A0=C2=A0 u8 nbytes; > =C2=A0=C2=A0=C2=A0=C2=A0 int ret; >=20 > @@ -438,12 +438,10 @@ static int sun4i_spi_xfer(struct udevice *dev,=20 > unsigned int bitlen, > =C2=A0=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 setbits_le32(SPI_REG(priv, S= PI_TCR), > =C2=A0=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 =C2=A0=C2= =A0=C2=A0=C2=A0 SPI_BIT(priv, SPI_TCR_XCH)); >=20 > -=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 /* Wait till RX FIFO to be empty */ > -=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 ret =3D readl_poll_timeout(SPI_REG= (priv, SPI_FSR), > -=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2= =A0 =C2=A0=C2=A0=C2=A0 =C2=A0rx_fifocnt, > -=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2= =A0 =C2=A0=C2=A0=C2=A0 =C2=A0(((rx_fifocnt & > -=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2= =A0 =C2=A0=C2=A0=C2=A0 =C2=A0SPI_BIT(priv, SPI_FSR_RF_CNT_MASK)) >> > -=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2= =A0 =C2=A0=C2=A0=C2=A0 =C2=A0SUN4I_FIFO_STA_RF_CNT_BITS) >=3D nbytes), > +=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 /* Wait untill transfer done */ > +=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 ret =3D readl_poll_timeout(SPI_REG= (priv, SPI_TCR), > +=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2= =A0 =C2=A0=C2=A0=C2=A0 =C2=A0tcr, > +=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2= =A0 =C2=A0=C2=A0=C2=A0 =C2=A0(!(tcr & SPI_BIT(priv, SPI_TCR_XCH))), > =C2=A0=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 =C2=A0=C2= =A0=C2=A0 =C2=A0=C2=A0=C2=A0 =C2=A0SUN4I_SPI_TIMEOUT_US); > =C2=A0=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 if (ret < 0) { > =C2=A0=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 printf("E= RROR: sun4i_spi: Timeout transferring data\n");