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 D1427C43334 for ; Sat, 2 Jul 2022 03:10:31 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 54E6F84382; Sat, 2 Jul 2022 05:10:29 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=163.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; unprotected) header.d=163.com header.i=@163.com header.b="UmtPW0oF"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 528E58439F; Sat, 2 Jul 2022 05:10:28 +0200 (CEST) Received: from m12-18.163.com (m12-18.163.com [220.181.12.18]) by phobos.denx.de (Postfix) with ESMTP id 73A49839E6 for ; Sat, 2 Jul 2022 05:10:24 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=163.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=qianfanguijin@163.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=163.com; s=s110527; h=Message-ID:Date:MIME-Version:Subject:From; bh=pMHuv ryiya7SwWWEMPjUsXBbRCFteH7Gg9dw8NQGL7c=; b=UmtPW0oFI6u7avsVboKeJ N/akLqGzgPEc66RH41CuNPCn2ZkI0IeAywm8BYC2z/mU7xEktP3X9MrKa+CrcuHN l1x4zEruyTfq/vvEniOUw9zCzIujwVCpOM1BdOfAt7hD5o0fYtzAtKit0I/zeMgZ pvTTzct2NRvCsK7mAa7L6M= Received: from [192.168.3.100] (unknown [218.201.129.20]) by smtp14 (Coremail) with SMTP id EsCowADnFcLxtr9ionf2Kg--.16742S2; Sat, 02 Jul 2022 11:09:37 +0800 (CST) Message-ID: <971f87b7-772f-e757-3e9b-cafc413dbd40@163.com> Date: Sat, 2 Jul 2022 11:09:37 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 Subject: Re: [PATCH v1] drivers: spi: sunxi: Fix spi speed settting Content-Language: en-US To: Andre Przywara , Jagan Teki Cc: u-boot@lists.denx.de, Chen-Yu Tsai , Maxime Ripard , Samuel Holland , Simon Glass References: <20220609090939.25828-1-qianfanguijin@163.com> <20220628013451.4d452a16@slackpad.lan> From: qianfan In-Reply-To: <20220628013451.4d452a16@slackpad.lan> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-CM-TRANSID: EsCowADnFcLxtr9ionf2Kg--.16742S2 X-Coremail-Antispam: 1Uf129KBjvJXoW3Ar47Ar1UKr4xKrWDZw1xAFb_yoW3XrWDpF Z5AF47AF4FqrnxtFn7Zr4UWrn0yF45uF4UCF1ftF4kArnI9FnxGF1jgFyfJFWxuFnFqa4r AF1UZFn5GF4kZaDanT9S1TB71UUUUUUqnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2 9KBjDUYxBIdaVFxhVjvjDU0xZFpf9x07jj6wZUUUUU= X-Originating-IP: [218.201.129.20] X-CM-SenderInfo: htld0w5dqj3xxmlqqiywtou0bp/1tbiXBgy7VXl3kCSWAAAs- 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 在 2022/6/28 8:34, Andre Przywara 写道: > On Thu, 9 Jun 2022 17:09:39 +0800 > qianfanguijin@163.com wrote: > > Hi Qianfan, > >> From: qianfan Zhao >> >> dm_spi_claim_bus run spi_set_speed_mode first and then ops->claim_bus, >> but spi clock is enabled when sun4i_spi_claim_bus, that will make >> sun4i_spi_set_speed doesn't work. > 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/ > > 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 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. 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 bus node: static int sun4i_spi_of_to_plat(struct udevice *bus) {     struct sun4i_spi_plat *plat = dev_get_plat(bus);     int node = dev_of_offset(bus);     plat->base = dev_read_addr(bus);     plat->variant = (struct sun4i_spi_variant *)dev_get_driver_data(bus);     plat->max_hz = fdtdec_get_int(gd->fdt_blob, node,                       "spi-max-frequency",                       SUN4I_SPI_DEFAULT_RATE);     if (plat->max_hz > SUN4I_SPI_MAX_RATE)         plat->max_hz = SUN4I_SPI_MAX_RATE;     return 0; } Seems this is not a correct way. "spi-max-frequency" should reading from spi device, not spi bus. On my dts, no "spi-max-frequency" prop on spi bus node, this will make plat->max_hz has default SUN4I_SPI_DEFAULT_RATE(1M) value. &spi2 {     pinctrl-names = "default";     pinctrl-0 = <&spi2_cs0_pb_pin &spi2_pb_pins>;     status = "okay";     lcd@0 {         compatible = "sitronix,st75161";         spi-max-frequency = <12000000>;         reg = <0>;         spi-cpol;         spi-cpha; So on my patch, I had changed the default plat->max_hz to 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 = 1(freq = 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 <= 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 = 6M: CCR: 00001001 ERROR: sun4i_spi: Timeout transferring data ERROR: sun4i_spi: Timeout transferring data ERROR: sun4i_spi: Timeout transferring data ... spi clock = 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 ... > > Thanks! > Andre > >> Fix it. >> >> Signed-off-by: qianfan Zhao >> --- >> drivers/spi/spi-sunxi.c | 78 ++++++++++++++++------------------------- >> 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: >> static int sun4i_spi_claim_bus(struct udevice *dev) >> { >> struct sun4i_spi_priv *priv = dev_get_priv(dev->parent); >> + u32 div, reg; >> int ret; >> >> ret = sun4i_spi_set_clock(dev->parent, true); >> @@ -233,12 +234,38 @@ static int sun4i_spi_claim_bus(struct udevice *dev) >> setbits_le32(SPI_REG(priv, SPI_GCR), SUN4I_CTL_ENABLE | >> SUN4I_CTL_MASTER | SPI_BIT(priv, SPI_GCR_TP)); >> >> + /* Setup clock divider */ >> + div = SUN4I_SPI_MAX_RATE / (2 * priv->freq); >> + reg = readl(SPI_REG(priv, SPI_CCR)); >> + >> + if (div <= (SUN4I_CLK_CTL_CDR2_MASK + 1)) { >> + if (div > 0) >> + div--; >> + >> + reg &= ~(SUN4I_CLK_CTL_CDR2_MASK | SUN4I_CLK_CTL_DRS); >> + reg |= SUN4I_CLK_CTL_CDR2(div) | SUN4I_CLK_CTL_DRS; >> + } else { >> + div = __ilog2(SUN4I_SPI_MAX_RATE) - __ilog2(priv->freq); >> + reg &= ~((SUN4I_CLK_CTL_CDR1_MASK << 8) | SUN4I_CLK_CTL_DRS); >> + reg |= SUN4I_CLK_CTL_CDR1(div); >> + } >> + >> + writel(reg, SPI_REG(priv, SPI_CCR)); >> + >> if (priv->variant->has_soft_reset) >> setbits_le32(SPI_REG(priv, SPI_GCR), >> SPI_BIT(priv, SPI_GCR_SRST)); >> >> - setbits_le32(SPI_REG(priv, SPI_TCR), SPI_BIT(priv, SPI_TCR_CS_MANUAL) | >> - SPI_BIT(priv, SPI_TCR_CS_ACTIVE_LOW)); >> + /* Setup the transfer control register */ >> + reg = SPI_BIT(priv, SPI_TCR_CS_MANUAL) | >> + SPI_BIT(priv, SPI_TCR_CS_ACTIVE_LOW); >> + >> + if (priv->mode & SPI_CPOL) >> + reg |= SPI_BIT(priv, SPI_TCR_CPOL); >> + if (priv->mode & SPI_CPHA) >> + reg |= SPI_BIT(priv, SPI_TCR_CPHA); >> + >> + writel(reg, SPI_REG(priv, SPI_TCR)); >> >> return 0; >> } >> @@ -329,67 +356,22 @@ static int sun4i_spi_set_speed(struct udevice *dev, uint speed) >> { >> struct sun4i_spi_plat *plat = dev_get_plat(dev); >> struct sun4i_spi_priv *priv = dev_get_priv(dev); >> - unsigned int div; >> - u32 reg; >> >> if (speed > plat->max_hz) >> speed = plat->max_hz; >> >> if (speed < SUN4I_SPI_MIN_RATE) >> speed = SUN4I_SPI_MIN_RATE; >> - /* >> - * Setup clock divider. >> - * >> - * We have two choices there. Either we can use the clock >> - * divide rate 1, which is calculated thanks to this formula: >> - * SPI_CLK = MOD_CLK / (2 ^ (cdr + 1)) >> - * Or we can use CDR2, which is calculated with the formula: >> - * SPI_CLK = MOD_CLK / (2 * (cdr + 1)) >> - * Whether we use the former or the latter is set through the >> - * DRS bit. >> - * >> - * First try CDR2, and if we can't reach the expected >> - * frequency, fall back to CDR1. >> - */ >> - >> - div = SUN4I_SPI_MAX_RATE / (2 * speed); >> - reg = readl(SPI_REG(priv, SPI_CCR)); >> - >> - if (div <= (SUN4I_CLK_CTL_CDR2_MASK + 1)) { >> - if (div > 0) >> - div--; >> - >> - reg &= ~(SUN4I_CLK_CTL_CDR2_MASK | SUN4I_CLK_CTL_DRS); >> - reg |= SUN4I_CLK_CTL_CDR2(div) | SUN4I_CLK_CTL_DRS; >> - } else { >> - div = __ilog2(SUN4I_SPI_MAX_RATE) - __ilog2(speed); >> - reg &= ~((SUN4I_CLK_CTL_CDR1_MASK << 8) | SUN4I_CLK_CTL_DRS); >> - reg |= SUN4I_CLK_CTL_CDR1(div); >> - } >> >> priv->freq = speed; >> - writel(reg, SPI_REG(priv, SPI_CCR)); >> - >> return 0; >> } >> >> static int sun4i_spi_set_mode(struct udevice *dev, uint mode) >> { >> struct sun4i_spi_priv *priv = dev_get_priv(dev); >> - u32 reg; >> - >> - reg = readl(SPI_REG(priv, SPI_TCR)); >> - reg &= ~(SPI_BIT(priv, SPI_TCR_CPOL) | SPI_BIT(priv, SPI_TCR_CPHA)); >> - >> - if (mode & SPI_CPOL) >> - reg |= SPI_BIT(priv, SPI_TCR_CPOL); >> - >> - if (mode & SPI_CPHA) >> - reg |= SPI_BIT(priv, SPI_TCR_CPHA); >> >> priv->mode = mode; >> - writel(reg, SPI_REG(priv, SPI_TCR)); >> - >> return 0; >> } >> >> @@ -441,7 +423,7 @@ static int sun4i_spi_of_to_plat(struct udevice *bus) >> plat->variant = (struct sun4i_spi_variant *)dev_get_driver_data(bus); >> plat->max_hz = fdtdec_get_int(gd->fdt_blob, node, >> "spi-max-frequency", >> - SUN4I_SPI_DEFAULT_RATE); >> + SUN4I_SPI_MAX_RATE); >> >> if (plat->max_hz > SUN4I_SPI_MAX_RATE) >> plat->max_hz = SUN4I_SPI_MAX_RATE;