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 EFB12C43334 for ; Sat, 2 Jul 2022 07:51:11 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 19392839F6; Sat, 2 Jul 2022 09:51:09 +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="NqkqzEyr"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id AF24F8440C; Sat, 2 Jul 2022 09:51:07 +0200 (CEST) Received: from mail-m971.mail.163.com (mail-m971.mail.163.com [123.126.97.1]) by phobos.denx.de (Postfix) with ESMTP id 27E6981F4E for ; Sat, 2 Jul 2022 09:51:02 +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=K25QG dnf+3p+onyMYxtb2JOgD15VKxERH9LdHvn0Lfw=; b=NqkqzEyrOKs5pluo+HBb/ +bFmrB/maiYux2ku0Js4M3o6yMl7eJQzQubFbRhFP7revUFWVOdkFTiWhP0KMHdI i7GNvAQZXb2V35D2K1w6iGCTV4ZQu4eQ3h6fe9+kEx4QaGcZnggEDsT1498M78u9 Z6qHGzcVEy18e8ggwhuDFM= Received: from [172.20.10.9] (unknown [112.224.65.28]) by smtp1 (Coremail) with SMTP id GdxpCgCn3ZjK+L9isjR3MA--.22951S2; Sat, 02 Jul 2022 15:50:35 +0800 (CST) Message-ID: <4bd00f93-55b9-563c-4d0f-27041ff972ab@163.com> Date: Sat, 2 Jul 2022 15:50:35 +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 From: qianfan To: Andre Przywara , Jagan Teki Cc: u-boot@lists.denx.de, Chen-Yu Tsai , Maxime Ripard , Samuel Holland , Simon Glass , Jesse Taube References: <20220609090939.25828-1-qianfanguijin@163.com> <20220628013451.4d452a16@slackpad.lan> <971f87b7-772f-e757-3e9b-cafc413dbd40@163.com> In-Reply-To: <971f87b7-772f-e757-3e9b-cafc413dbd40@163.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-CM-TRANSID: GdxpCgCn3ZjK+L9isjR3MA--.22951S2 X-Coremail-Antispam: 1Uf129KBjvJXoW3WF18Xr4fAr1ftF4xWr13XFb_yoW3CFWDpF WrCF4UAa1FqrnxKFnrZF4jgr1YkFW5uF4UuFn3tF18ArnF9FnxGF1UGr1fGFWxuF17Xa4r AF1UuFn3GFs5ZaDanT9S1TB71UUUUUUqnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2 9KBjDUYxBIdaVFxhVjvjDU0xZFpf9x07j5qXdUUUUU= X-Originating-IP: [112.224.65.28] X-CM-SenderInfo: htld0w5dqj3xxmlqqiywtou0bp/xtbBzgsy7WI0VJH6xQAAs2 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/7/2 11:09, qianfan 写道: > > > 在 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 > ... Add udelay(1) before sun4i_spi_drain_fifo in sun4i_spi_xfer can fix it. But I don't know why. > >> >> 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; >