lkml.org 
[lkml]   [2022]   [Mar]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v4 08/11] spi: aspeed: Calibrate read timings
From
On 3/30/22 13:53, Chin-Ting Kuo wrote:
> Hi Cédric Le,
>
>> -----Original Message-----
>> From: Cédric Le Goater <clg@kaod.org>
>> Sent: Friday, March 25, 2022 6:09 PM
>> To: linux-spi@vger.kernel.org; linux-mtd@lists.infradead.org
>> Subject: [PATCH v4 08/11] spi: aspeed: Calibrate read timings
>>
>> To accommodate the different response time of SPI transfers on different
>> boards and different SPI NOR devices, the Aspeed controllers provide a set of
>> Read Timing Compensation registers to tune the timing delays depending on
>> the frequency being used. The AST2600 SoC has one of these registers per
>> device. On the AST2500 and AST2400 SoCs, the timing register is shared by all
>> devices which is problematic to get good results other than for one device.
>>
>> The algorithm first reads a golden buffer at low speed and then performs reads
>> with different clocks and delay cycle settings to find a breaking point. This
>> selects a default good frequency for the CEx control register.
>> The current settings are a bit optimistic as we pick the first delay giving good
>> results. A safer approach would be to determine an interval and choose the
>> middle value.
>>
>> Calibration is performed when the direct mapping for reads is created.
>> Since the underlying spi-nor object needs to be initialized to create the
>> spi_mem operation for direct mapping, we should be fine. Having a specific
>> API would clarify the requirements though.
>>
>> Cc: Pratyush Yadav <p.yadav@ti.com>
>> Reviewed-by: Joel Stanley <joel@jms.id.au>
>> Tested-by: Joel Stanley <joel@jms.id.au>
>> Tested-by: Tao Ren <rentao.bupt@gmail.com>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>> drivers/spi/spi-aspeed-smc.c | 281
>> +++++++++++++++++++++++++++++++++++
>> 1 file changed, 281 insertions(+)
>>
>> diff --git a/drivers/spi/spi-aspeed-smc.c b/drivers/spi/spi-aspeed-smc.c index
>> 7f306da7c44e..660451667a39 100644
>> --- a/drivers/spi/spi-aspeed-smc.c
>> +++ b/drivers/spi/spi-aspeed-smc.c
>> @@ -33,6 +33,8 @@
>> #define CTRL_IO_ADDRESS_4B BIT(13) /* AST2400 SPI only */
>> #define CTRL_IO_DUMMY_SET(dummy) \
>> (((((dummy) >> 2) & 0x1) << 14) | (((dummy) & 0x3) << 6))
>> +#define CTRL_FREQ_SEL_SHIFT 8
>> +#define CTRL_FREQ_SEL_MASK GENMASK(11,
>> CTRL_FREQ_SEL_SHIFT)
>> #define CTRL_CE_STOP_ACTIVE BIT(2)
>> #define CTRL_IO_MODE_CMD_MASK GENMASK(1, 0)
>> #define CTRL_IO_MODE_NORMAL 0x0
>> @@ -45,6 +47,9 @@
>> /* CEx Address Decoding Range Register */
>> #define CE0_SEGMENT_ADDR_REG 0x30
>>
>> +/* CEx Read timing compensation register */
>> +#define CE0_TIMING_COMPENSATION_REG 0x94
>> +
>> enum aspeed_spi_ctl_reg_value {
>> ASPEED_SPI_BASE,
>> ASPEED_SPI_READ,
>> @@ -70,10 +75,15 @@ struct aspeed_spi_data {
>> bool hastype;
>> u32 mode_bits;
>> u32 we0;
>> + u32 timing;
>> + u32 hclk_mask;
>> + u32 hdiv_max;
>>
>> u32 (*segment_start)(struct aspeed_spi *aspi, u32 reg);
>> u32 (*segment_end)(struct aspeed_spi *aspi, u32 reg);
>> u32 (*segment_reg)(struct aspeed_spi *aspi, u32 start, u32 end);
>> + int (*calibrate)(struct aspeed_spi_chip *chip, u32 hdiv,
>> + const u8 *golden_buf, u8 *test_buf);
>> };
>>
>> #define ASPEED_SPI_MAX_NUM_CS 5
>> @@ -517,6 +527,8 @@ static int aspeed_spi_chip_adjust_window(struct
>> aspeed_spi_chip *chip,
>> return 0;
>> }
>>
>> +static int aspeed_spi_do_calibration(struct aspeed_spi_chip *chip);
>> +
>> static int aspeed_spi_dirmap_create(struct spi_mem_dirmap_desc *desc) {
>> struct aspeed_spi *aspi =
>> spi_controller_get_devdata(desc->mem->spi->master);
>> @@ -565,6 +577,8 @@ static int aspeed_spi_dirmap_create(struct
>> spi_mem_dirmap_desc *desc)
>> chip->ctl_val[ASPEED_SPI_READ] = ctl_val;
>> writel(chip->ctl_val[ASPEED_SPI_READ], chip->ctl);
>>
>> + ret = aspeed_spi_do_calibration(chip);
>> +
>> dev_info(aspi->dev, "CE%d read buswidth:%d [0x%08x]\n",
>> chip->cs, op->data.buswidth, chip->ctl_val[ASPEED_SPI_READ]);
>>
>> @@ -812,6 +826,249 @@ static u32 aspeed_spi_segment_ast2600_reg(struct
>> aspeed_spi *aspi,
>> ((end - 1) & AST2600_SEG_ADDR_MASK);
>> }
>>
>> +/*
>> + * Read timing compensation sequences
>> + */
>> +
>> +#define CALIBRATE_BUF_SIZE SZ_16K
>> +
>> +static bool aspeed_spi_check_reads(struct aspeed_spi_chip *chip,
>> + const u8 *golden_buf, u8 *test_buf) {
>> + int i;
>> +
>> + for (i = 0; i < 10; i++) {
>> + memcpy_fromio(test_buf, chip->ahb_base, CALIBRATE_BUF_SIZE);
>> + if (memcmp(test_buf, golden_buf, CALIBRATE_BUF_SIZE) != 0) { #if
>> +defined(VERBOSE_DEBUG)
>> + print_hex_dump_bytes(DEVICE_NAME " fail: ",
>> DUMP_PREFIX_NONE,
>> + test_buf, 0x100);
>> +#endif
>> + return false;
>> + }
>> + }
>> + return true;
>> +}
>> +
>> +#define FREAD_TPASS(i) (((i) / 2) | (((i) & 1) ? 0 : 8))
>> +
>> +/*
>> + * The timing register is shared by all devices. Only update for CE0.
>> + */
>> +static int aspeed_spi_calibrate(struct aspeed_spi_chip *chip, u32 hdiv,
>> + const u8 *golden_buf, u8 *test_buf) {
>> + struct aspeed_spi *aspi = chip->aspi;
>> + const struct aspeed_spi_data *data = aspi->data;
>> + int i;
>> + int good_pass = -1, pass_count = 0;
>> + u32 shift = (hdiv - 1) << 2;
>> + u32 mask = ~(0xfu << shift);
>> + u32 fread_timing_val = 0;
>> +
>> + /* Try HCLK delay 0..5, each one with/without delay and look for a
>> + * good pair.
>> + */
>> + for (i = 0; i < 12; i++) {
>> + bool pass;
>> +
>> + if (chip->cs == 0) {
>> + fread_timing_val &= mask;
>> + fread_timing_val |= FREAD_TPASS(i) << shift;
>> + writel(fread_timing_val, aspi->regs + data->timing);
>> + }
>> + pass = aspeed_spi_check_reads(chip, golden_buf, test_buf);
>> + dev_dbg(aspi->dev,
>> + " * [%08x] %d HCLK delay, %dns DI delay : %s",
>> + fread_timing_val, i / 2, (i & 1) ? 0 : 4,
>> + pass ? "PASS" : "FAIL");
>> + if (pass) {
>> + pass_count++;
>> + if (pass_count == 3) {
>> + good_pass = i - 1;
>> + break;
>> + }
>> + } else {
>> + pass_count = 0;
>> + }
>> + }
>> +
>> + /* No good setting for this frequency */
>> + if (good_pass < 0)
>> + return -1;
>> +
>> + /* We have at least one pass of margin, let's use first pass */
>> + if (chip->cs == 0) {
>> + fread_timing_val &= mask;
>> + fread_timing_val |= FREAD_TPASS(good_pass) << shift;
>> + writel(fread_timing_val, aspi->regs + data->timing);
>> + }
>> + dev_dbg(aspi->dev, " * -> good is pass %d [0x%08x]",
>> + good_pass, fread_timing_val);
>> + return 0;
>> +}
>> +
>> +static bool aspeed_spi_check_calib_data(const u8 *test_buf, u32 size) {
>> + const u32 *tb32 = (const u32 *)test_buf;
>> + u32 i, cnt = 0;
>> +
>> + /* We check if we have enough words that are neither all 0
>> + * nor all 1's so the calibration can be considered valid.
>> + *
>> + * I use an arbitrary threshold for now of 64
>> + */
>> + size >>= 2;
>> + for (i = 0; i < size; i++) {
>> + if (tb32[i] != 0 && tb32[i] != 0xffffffff)
>> + cnt++;
>> + }
>> + return cnt >= 64;
>> +}
>> +
>> +static const u32 aspeed_spi_hclk_divs[] = {
>> + 0xf, /* HCLK */
>> + 0x7, /* HCLK/2 */
>> + 0xe, /* HCLK/3 */
>> + 0x6, /* HCLK/4 */
>> + 0xd, /* HCLK/5 */
>> +};
>> +
>> +#define ASPEED_SPI_HCLK_DIV(i) \
>> + (aspeed_spi_hclk_divs[(i) - 1] << CTRL_FREQ_SEL_SHIFT)
>> +
>> +static int aspeed_spi_do_calibration(struct aspeed_spi_chip *chip) {
>> + struct aspeed_spi *aspi = chip->aspi;
>> + const struct aspeed_spi_data *data = aspi->data;
>> + u32 ahb_freq = aspi->clk_freq;
>> + u32 max_freq = chip->clk_freq;
>> + u32 ctl_val;
>> + u8 *golden_buf = NULL;
>> + u8 *test_buf = NULL;
>> + int i, rc, best_div = -1;
>> +
>> + dev_dbg(aspi->dev, "calculate timing compensation - AHB freq: %d MHz",
>> + ahb_freq / 1000000);
>> +
>> + /*
>> + * use the related low frequency to get check calibration data
>> + * and get golden data.
>> + */
>> + ctl_val = chip->ctl_val[ASPEED_SPI_READ] & data->hclk_mask;
>> + writel(ctl_val, chip->ctl);
>> +
>> + test_buf = kzalloc(CALIBRATE_BUF_SIZE * 2, GFP_KERNEL);
>> + if (!test_buf)
>> + return -ENOMEM;
>> +
>> + golden_buf = test_buf + CALIBRATE_BUF_SIZE;
>> +
>> + memcpy_fromio(golden_buf, chip->ahb_base, CALIBRATE_BUF_SIZE);
>> + if (!aspeed_spi_check_calib_data(golden_buf, CALIBRATE_BUF_SIZE)) {
>> + dev_info(aspi->dev, "Calibration area too uniform, using low speed");
>> + goto no_calib;
>> + }
>> +
>> +#if defined(VERBOSE_DEBUG)
>> + print_hex_dump_bytes(DEVICE_NAME " good: ", DUMP_PREFIX_NONE,
>> + golden_buf, 0x100);
>> +#endif
>> +
>> + /* Now we iterate the HCLK dividers until we find our breaking point */
>> + for (i = ARRAY_SIZE(aspeed_spi_hclk_divs); i > data->hdiv_max - 1; i--) {
>> + u32 tv, freq;
>> +
>> + freq = ahb_freq / i;
>> + if (freq > max_freq)
>> + continue;
>> +
>> + /* Set the timing */
>> + tv = chip->ctl_val[ASPEED_SPI_READ] | ASPEED_SPI_HCLK_DIV(i);
>> + writel(tv, chip->ctl);
>> + dev_dbg(aspi->dev, "Trying HCLK/%d [%08x] ...", i, tv);
>> + rc = data->calibrate(chip, i, golden_buf, test_buf);
>> + if (rc == 0)
>> + best_div = i;
>> + }
>> +
>> + /* Nothing found ? */
>> + if (best_div < 0) {
>> + dev_warn(aspi->dev, "No good frequency, using dumb slow");
>> + } else {
>> + dev_dbg(aspi->dev, "Found good read timings at HCLK/%d",
>> best_div);
>> +
>> + /* Record the freq */
>> + for (i = 0; i < ASPEED_SPI_MAX; i++)
>> + chip->ctl_val[i] = (chip->ctl_val[i] & data->hclk_mask) |
>> + ASPEED_SPI_HCLK_DIV(best_div);
>> + }
>> +
>> +no_calib:
>
> - Maybe, if the calibration process is not executed, the frequency setting calculated from max_frequency in the device tree can be filled in FMC10 instead of using dumb slow one, 12.5MHz, always.

Indeed.

> For example, except for uniform content case, the calibration process will be ignored when SPI clock frequency in the device tree is smaller than 40MHz.
> - The function, aspeed_2600_spi_clk_basic_setting, in [2] can be added to support lower SPI clock frequency, e.g., 4MHz.
> For AST2600, SPI clock frequency can be calculated by HCLK/(FMC10[27:24] + FMC10[11:8]).

Could you please send patches on top of this series ? Here are the branches :

https://github.com/legoater/linux/commits/openbmc-5.15
https://github.com/legoater/linux/commits/aspeed (mainline)

I can include them when I resend a v5.

Thanks,

C.


>
>> + writel(chip->ctl_val[ASPEED_SPI_READ], chip->ctl);
>> + kfree(test_buf);
>> + return 0;
>> +}
>> +
>> +#define TIMING_DELAY_DI BIT(3)
>> +#define TIMING_DELAY_HCYCLE_MAX 5
>> +#define TIMING_REG_AST2600(chip) \
>> + ((chip)->aspi->regs + (chip)->aspi->data->timing + \
>> + (chip)->cs * 4)
>> +
>> +static int aspeed_spi_ast2600_calibrate(struct aspeed_spi_chip *chip, u32
>> hdiv,
>> + const u8 *golden_buf, u8 *test_buf) {
>> + struct aspeed_spi *aspi = chip->aspi;
>> + int hcycle;
>> + u32 shift = (hdiv - 2) << 3;
>> + u32 mask = ~(0xfu << shift);
>> + u32 fread_timing_val = 0;
>> +
>> + for (hcycle = 0; hcycle <= TIMING_DELAY_HCYCLE_MAX; hcycle++) {
>> + int delay_ns;
>> + bool pass = false;
>> +
>> + fread_timing_val &= mask;
>> + fread_timing_val |= hcycle << shift;
>> +
>> + /* no DI input delay first */
>> + writel(fread_timing_val, TIMING_REG_AST2600(chip));
>> + pass = aspeed_spi_check_reads(chip, golden_buf, test_buf);
>> + dev_dbg(aspi->dev,
>> + " * [%08x] %d HCLK delay, DI delay none : %s",
>> + fread_timing_val, hcycle, pass ? "PASS" : "FAIL");
>> + if (pass)
>> + return 0;
>> +
>> + /* Add DI input delays */
>> + fread_timing_val &= mask;
>> + fread_timing_val |= (TIMING_DELAY_DI | hcycle) << shift;
>> +
>> + for (delay_ns = 0; delay_ns < 0x10; delay_ns++) {
>> + fread_timing_val &= ~(0xf << (4 + shift));
>> + fread_timing_val |= delay_ns << (4 + shift);
>> +
>> + writel(fread_timing_val, TIMING_REG_AST2600(chip));
>> + pass = aspeed_spi_check_reads(chip, golden_buf, test_buf);
>> + dev_dbg(aspi->dev,
>> + " * [%08x] %d HCLK delay, DI delay %d.%dns : %s",
>> + fread_timing_val, hcycle, (delay_ns + 1) / 2,
>> + (delay_ns + 1) & 1 ? 5 : 5, pass ? "PASS" : "FAIL");
>> + /*
>> + * TODO: This is optimistic. We should look
>> + * for a working interval and save the middle
>> + * value in the read timing register.
>> + */
>> + if (pass)
>> + return 0;
>> + }
>> + }
>> +
>> + /* No good setting for this frequency */
>> + return -1;
>> +}
>> +
>> /*
>> * Platform definitions
>> */
>> @@ -820,6 +1077,10 @@ static const struct aspeed_spi_data
>> ast2400_fmc_data = {
>> .hastype = true,
>> .we0 = 16,
>> .ctl0 = CE0_CTRL_REG,
>> + .timing = CE0_TIMING_COMPENSATION_REG,
>> + .hclk_mask = 0xfffff0ff,
>> + .hdiv_max = 1,
>> + .calibrate = aspeed_spi_calibrate,
>> .segment_start = aspeed_spi_segment_start,
>> .segment_end = aspeed_spi_segment_end,
>> .segment_reg = aspeed_spi_segment_reg,
>> @@ -830,6 +1091,10 @@ static const struct aspeed_spi_data
>> ast2400_spi_data = {
>> .hastype = false,
>> .we0 = 0,
>> .ctl0 = 0x04,
>> + .timing = 0x14,
>> + .hclk_mask = 0xfffff0ff,
>> + .hdiv_max = 1,
>> + .calibrate = aspeed_spi_calibrate,
>> /* No segment registers */
>> };
>>
>> @@ -838,6 +1103,10 @@ static const struct aspeed_spi_data
>> ast2500_fmc_data = {
>> .hastype = true,
>> .we0 = 16,
>> .ctl0 = CE0_CTRL_REG,
>> + .timing = CE0_TIMING_COMPENSATION_REG,
>> + .hclk_mask = 0xfffff0ff,
>> + .hdiv_max = 1,
>> + .calibrate = aspeed_spi_calibrate,
>> .segment_start = aspeed_spi_segment_start,
>> .segment_end = aspeed_spi_segment_end,
>> .segment_reg = aspeed_spi_segment_reg,
>> @@ -848,6 +1117,10 @@ static const struct aspeed_spi_data
>> ast2500_spi_data = {
>> .hastype = false,
>> .we0 = 16,
>> .ctl0 = CE0_CTRL_REG,
>> + .timing = CE0_TIMING_COMPENSATION_REG,
>> + .hclk_mask = 0xfffff0ff,
>> + .hdiv_max = 1,
>> + .calibrate = aspeed_spi_calibrate,
>> .segment_start = aspeed_spi_segment_start,
>> .segment_end = aspeed_spi_segment_end,
>> .segment_reg = aspeed_spi_segment_reg,
>> @@ -859,6 +1132,10 @@ static const struct aspeed_spi_data
>> ast2600_fmc_data = {
>> .mode_bits = SPI_RX_QUAD | SPI_RX_QUAD,
>> .we0 = 16,
>> .ctl0 = CE0_CTRL_REG,
>> + .timing = CE0_TIMING_COMPENSATION_REG,
>> + .hclk_mask = 0xf0fff0ff,
>> + .hdiv_max = 2,
>> + .calibrate = aspeed_spi_ast2600_calibrate,
>> .segment_start = aspeed_spi_segment_ast2600_start,
>> .segment_end = aspeed_spi_segment_ast2600_end,
>> .segment_reg = aspeed_spi_segment_ast2600_reg,
>> @@ -870,6 +1147,10 @@ static const struct aspeed_spi_data
>> ast2600_spi_data = {
>> .mode_bits = SPI_RX_QUAD | SPI_RX_QUAD,
>> .we0 = 16,
>> .ctl0 = CE0_CTRL_REG,
>> + .timing = CE0_TIMING_COMPENSATION_REG,
>> + .hclk_mask = 0xf0fff0ff,
>> + .hdiv_max = 2,
>> + .calibrate = aspeed_spi_ast2600_calibrate,
>> .segment_start = aspeed_spi_segment_ast2600_start,
>> .segment_end = aspeed_spi_segment_ast2600_end,
>> .segment_reg = aspeed_spi_segment_ast2600_reg,
>> --
>> 2.34.1
>
>
> Best Wishes,
> Chin-Ting

\
 
 \ /
  Last update: 2022-03-30 15:01    [W:0.120 / U:1.600 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site