[PATCH v1] drivers: spi: sunxi: Fix spi speed settting
qianfan
qianfanguijin at 163.com
Sat Jul 2 10:20:37 CEST 2022
在 2022/7/2 15:50, qianfan 写道:
>
>
> 在 2022/7/2 11:09, qianfan 写道:
>>
>>
>> 在 2022/6/28 8:34, Andre Przywara 写道:
>>> On Thu, 9 Jun 2022 17:09:39 +0800
>>> qianfanguijin at 163.com wrote:
>>>
>>> Hi Qianfan,
>>>
>>>> From: qianfan Zhao <qianfanguijin at 163.com>
>>>>
>>>> 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 at 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 <qianfanguijin at 163.com>
>>>> ---
>>>> 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;
>>
>
Hi everyone:
I had fixed "Timeout transferring data" issue and tested on sun8i r40
platform.
But I don't have a SUN4I_A10 board, could you please test and check this
patch?
From 514e9396509593515b7fa848cbc4b8eccf948547 Mon Sep 17 00:00:00 2001
From: qianfan Zhao <qianfanguijin at 163.com>
Date: Sat, 2 Jul 2022 16:07:18 +0800
Subject: [PATCH] spi: sunxi: Fix transfer timeout when running at a low
frequency
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
Fix the waiting condition.
Signed-off-by: qianfan Zhao <qianfanguijin at 163.com>
---
drivers/spi/spi-sunxi.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
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,
unsigned int bitlen,
struct dm_spi_slave_plat *slave_plat = dev_get_parent_plat(dev);
u32 len = bitlen / 8;
- u32 rx_fifocnt;
+ u32 tcr;
u8 nbytes;
int ret;
@@ -438,12 +438,10 @@ static int sun4i_spi_xfer(struct udevice *dev,
unsigned int bitlen,
setbits_le32(SPI_REG(priv, SPI_TCR),
SPI_BIT(priv, SPI_TCR_XCH));
- /* Wait till RX FIFO to be empty */
- ret = readl_poll_timeout(SPI_REG(priv, SPI_FSR),
- rx_fifocnt,
- (((rx_fifocnt &
- SPI_BIT(priv, SPI_FSR_RF_CNT_MASK)) >>
- SUN4I_FIFO_STA_RF_CNT_BITS) >= nbytes),
+ /* Wait untill transfer done */
+ ret = readl_poll_timeout(SPI_REG(priv, SPI_TCR),
+ tcr,
+ (!(tcr & SPI_BIT(priv, SPI_TCR_XCH))),
SUN4I_SPI_TIMEOUT_US);
if (ret < 0) {
printf("ERROR: sun4i_spi: Timeout transferring data\n");
--
2.25.1
More information about the U-Boot
mailing list