[PATCH v1] drivers: spi: sunxi: Fix spi speed settting

qianfan qianfanguijin at 163.com
Sat Jul 2 09:50:35 CEST 2022



在 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;
>



More information about the U-Boot mailing list