[U-Boot] [PATCH 2/3] spi: Add support for the Aspeed ast2500 SPI controllers

[Joel Stanley]

On Mon, 10 Sep 2018 at 23:48, Cédric Le Goater <clg at kaod.org> wrote:
>
> The Aspeed AST2500 SoC comes with three static memory controllers, all
> with a similar interface :
>
>  * Firmware SPI Memory Controller (FMC)
>    . BMC firmware
>    . 3 chip select pins (CE0 ~ CE2)
>    . supports SPI type flash memory (CE0 ~ CE1)
>    . CE2 can be of NOR type flash but this is not supported by the
>      driver
>
>  * SPI Flash Controller (SPI1 and SPI2)
>    . host firmware
>    . 2 chip select pins (CE0 ~ CE1)
>
> Each controller has a defined AHB window for its registers and another
> AHB window on which all the flash devices are mapped. Each device is
> assigned a memory range through a set of registers called the Segment
> Address Registers.
>
> Devices are controlled using two different modes: the USER command
> mode or the READ/WRITE command mode. When in USER command mode,
> accesses to the AHB window of the SPI flash device are translated into
> SPI command transfers. When in READ/WRITE command mode, the HW
> generates the SPI commands depending on the setting of the CE control
> register.
>
> The following driver supports the FMC and the SPI controllers with the
> attached SPI flash devices. When the controller is probed, the driver
> performs a read timing calibration using specific DMA control
> registers (FMC only). The driver's primary goal is to support the
> first device of the FMC controller on which reside U-Boot but it
> should support the other controllers also.
>
> The Aspeed FMC controller automatically detects at boot time if a
> flash device is in 4BYTE address mode and self configures to use the
> appropriate address width. This can be a problem for the U-Boot SPI
> Flash layer which only supports 3 byte addresses. In such a case, a
> warning is emitted and the address width is fixed when sent on the
> bus.
>
> Signed-off-by: Cédric Le Goater <clg at kaod.org>

Looks good. I compared some things against the data sheet, and read
through the driver. There were to small questions I had, so please
clear those up and add my:

Reviewed-by: Joel Stanley <joel at jms.id.au>

> +static u32 aspeed_spi_hclk_divisor(u32 hclk_rate, u32 max_hz)
> +{
> +       /* HCLK/1 ..    HCLK/16 */
> +       const u8 hclk_divisors[] = {
> +               15, 7, 14, 6, 13, 5, 12, 4, 11, 3, 10, 2, 9, 1, 8, 0
> +       };
> +
> +       u32 i;
> +
> +       for (i = 0; i < ARRAY_SIZE(hclk_divisors); i++) {
> +               if (max_hz >= (hclk_rate / (i + 1)))

We spoke offline about why the values in hclk_divisors are not used in
this calculation. I think we decided to change the name of
hclk_divisors to something like hclk_masks?

> +                       break;
> +       }
> +
> +       debug("hclk=%d required=%d divisor is %d (mask %x) speed=%d\n",
> +             hclk_rate, max_hz, i + 1, hclk_divisors[i], hclk_rate / (i + 1));
> +
> +       return hclk_divisors[i];
> +}

> +static u32 aspeed_spi_read_checksum(struct aspeed_spi_priv *priv, u8 div,
> +                                   u8 delay)
> +{
> +       /* TODO: the SPI controllers do not have the DMA registers.
> +        * The algorithm is the same.
> +        */
> +       if (!priv->is_fmc) {
> +               pr_warn("No timing calibration support for SPI controllers");
> +               return 0xbadc0de;
> +       }
> +
> +       return aspeed_spi_fmc_checksum(priv, div, delay);
> +}
> +
> +static int aspeed_spi_timing_calibration(struct aspeed_spi_priv *priv)
> +{
> +       /* HCLK/5 .. HCLK/1 */
> +       const u8 hclk_divisors[] = { 13, 6, 14, 7, 15 };
> +
> +       u32 timing_reg = 0;
> +       u32 checksum, gold_checksum;
> +       int i, j;
> +
> +       debug("Read timing calibration :\n");
> +
> +       /* Compute reference checksum at lowest freq HCLK/16 */
> +       gold_checksum = aspeed_spi_read_checksum(priv, 0, 0);
> +
> +       /*
> +        * Set CE0 Control Register to FAST READ command mode. The
> +        * HCLK divisor will be set through the DMA Control Register.
> +        */
> +       writel(CE_CTRL_CMD(0xB) | CE_CTRL_DUMMY(1) | CE_CTRL_FREADMODE,
> +              &priv->regs->ce_ctrl[0]);
> +
> +       /* Increase HCLK freq */
> +       for (i = 0; i < ARRAY_SIZE(hclk_divisors); i++) {
> +               u32 hdiv = 5 - i;
> +               u32 hshift = (hdiv - 1) << 2;
> +               bool pass = false;
> +               u8 delay;
> +
> +               if (priv->hclk_rate / hdiv > priv->max_hz) {
> +                       debug("skipping freq %ld\n", priv->hclk_rate / hdiv);
> +                       continue;
> +               }
> +
> +               /* Increase HCLK delays until read succeeds */
> +               for (j = 0; j < 6; j++) {

6 here is the number of items in hclk_divisors?

> +                       /* Try first with a 4ns DI delay */
> +                       delay = 0x8 | j;
> +                       checksum = aspeed_spi_read_checksum(
> +                               priv, hclk_divisors[i], delay);
> +                       pass = (checksum == gold_checksum);
> +                       debug(" HCLK/%d, 4ns DI delay, %d HCLK delay : %s\n",
> +                             hdiv, j, pass ? "PASS" : "FAIL");
> +
> +                       /* Try again with more HCLK delays */
> +                       if (!pass)
> +                               continue;
> +
> +                       /* Try without the 4ns DI delay */
> +                       delay = j;
> +                       checksum = aspeed_spi_read_checksum(
> +                               priv, hclk_divisors[i], delay);
> +                       pass = (checksum == gold_checksum);
> +                       debug(" HCLK/%d, 0ns DI delay, %d HCLK delay : %s\n",
> +                             hdiv, j, pass ? "PASS" : "FAIL");
> +
> +                       /* All good for this freq  */
> +                       if (pass)
> +                               break;
> +               }
> +
> +               if (pass) {
> +                       timing_reg &= ~(0xfu << hshift);
> +                       timing_reg |= delay << hshift;
> +               }
> +       }
> +
> +       debug("Timing Register set to 0x%08x\n", timing_reg);
> +       writel(timing_reg, &priv->regs->timings);
> +
> +       /* Reset CE0 Control Register */
> +       writel(0x0, &priv->regs->ce_ctrl[0]);
> +       return 0;
> +}
> +