[PATCH 1/1] spi: aspeed: Update SPI clock frequency configuration

Cédric Le Goater clg at kaod.org
Tue Apr 19 06:41:34 AEST 2022


Hello Chin-Ting,

On 4/14/22 12:28, Chin-Ting Kuo wrote:
> Instead of using the slowest one, the maximum clock
> frequency configured in the device tree should be kept
> when no timing calibration process is executed.
> Besides, an extra callback function is added in order
> to calculate clock frequency configuration for
> different ASPEED platforms.
> 
> Signed-off-by: Chin-Ting Kuo <chin-ting_kuo at aspeedtech.com>

I gave this patch a try on an AST2600 A3 EVB and an AST2500 EVB and it
behaved as expected. The default setting from the DT is chosen when the
flash contents are too uniform.


> ---
>   drivers/spi/spi-aspeed-smc.c | 166 +++++++++++++++++++++++++++++++----
>   1 file changed, 149 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/spi/spi-aspeed-smc.c b/drivers/spi/spi-aspeed-smc.c
> index 227797e13997..728163d0045d 100644
> --- a/drivers/spi/spi-aspeed-smc.c
> +++ b/drivers/spi/spi-aspeed-smc.c
> @@ -3,7 +3,7 @@
>    * ASPEED FMC/SPI Memory Controller Driver
>    *
>    * Copyright (c) 2015-2022, IBM Corporation.
> - * Copyright (c) 2020, ASPEED Corporation.
> + * Copyright (c) 2020-2022, ASPEED Corporation.
>    */
>   #include <linux/clk.h>
> @@ -84,6 +84,7 @@ struct aspeed_spi_data {
>   	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);
> +	u32 (*clk_config)(struct aspeed_spi_chip *chip, u32 max_hz);
>   };
>   
>   #define ASPEED_SPI_MAX_NUM_CS	5
> @@ -959,7 +960,10 @@ static int aspeed_spi_do_calibration(struct aspeed_spi_chip *chip)
>   	u32 ctl_val;
>   	u8 *golden_buf = NULL;
>   	u8 *test_buf = NULL;
> -	int i, rc, best_div = -1;
> +	int i, rc;
> +	u32 best_freq = 0;
> +	u32 freq;
> +	u32 clk_conf;
>   
>   	dev_dbg(aspi->dev, "calculate timing compensation - AHB freq: %d MHz",
>   		ahb_freq / 1000000);
> @@ -980,7 +984,7 @@ static int aspeed_spi_do_calibration(struct aspeed_spi_chip *chip)
>   	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");

may be change dev_info() to "..., using default freq"

> -		goto no_calib;
> +		goto end_calib;
>   	}
>   
>   #if defined(VERBOSE_DEBUG)
> @@ -990,7 +994,7 @@ static int aspeed_spi_do_calibration(struct aspeed_spi_chip *chip)
>   
>   	/* 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;
> +		u32 tv;
>   
>   		freq = ahb_freq / i;
>   		if (freq > max_freq)
> @@ -1002,27 +1006,149 @@ static int aspeed_spi_do_calibration(struct aspeed_spi_chip *chip)
>   		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;
> +			best_freq = freq;
>   	}
>   
>   	/* 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);
> +	if (best_freq == 0)
> +		dev_warn(aspi->dev, "Use the default timing setting");
> +	else
> +		dev_dbg(aspi->dev, "Found good read timings at HCLK/%d", i);
>   
> -		/* 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:
> +end_calib:
> +	if (best_freq == 0)
> +		best_freq = max_freq;
> +
> +	clk_conf = data->clk_config(chip, best_freq);
> +	/* Record the freq */
> +	for (i = 0; i < ASPEED_SPI_MAX; i++)
> +		chip->ctl_val[i] = (chip->ctl_val[i] & data->hclk_mask) | clk_conf;
> +
>   	writel(chip->ctl_val[ASPEED_SPI_READ], chip->ctl);
>   	kfree(test_buf);
>   	return 0;
>   }
>   
> +/* HCLK/1 ..	HCLK/16 */
> +static const u32 aspeed_spi_hclk_masks[] = {
> +	15, 7, 14, 6, 13, 5, 12, 4,
> +	11, 3, 10, 2, 9,  1, 8,  0
> +};

This new array is a bit redundant with aspeed_spi_hclk_divs[]

> +
> +static u32 aspeed_spi_ast2400_clk_config(struct aspeed_spi_chip *chip,
> +					 u32 max_hz)
> +{
> +	struct aspeed_spi *aspi = chip->aspi;
> +	u32 ahb_freq = aspi->clk_freq;
> +	u32 hclk_div = 0; /* default value */
> +	u32 i;
> +	bool found = false;
> +
> +	/* FMC/SPIR10[11:8] */
> +	for (i = 0; i < ARRAY_SIZE(aspeed_spi_hclk_masks); i++) {
> +		if (ahb_freq / (i + 1) <= max_hz) {
> +			found = true;
> +			break;
> +		}
> +	}
> +
> +	if (found)
> +		hclk_div = aspeed_spi_hclk_masks[i] << 8;
> +
> +	dev_dbg(aspi->dev, "found: %s, hclk: %d, max_clk: %d\n",
> +		found ? "yes" : "no", ahb_freq, max_hz);
> +
> +	if (found) {
> +		dev_dbg(aspi->dev, "h_div: %d (mask %x)\n",
> +			i + 1, aspeed_spi_hclk_masks[i]);
> +	}
> +
> +	return hclk_div;
> +}
> +
> +static u32 aspeed_spi_ast2500_clk_config(struct aspeed_spi_chip *chip,
> +					 u32 max_hz)
> +{
> +	struct aspeed_spi *aspi = chip->aspi;
> +	u32 ahb_freq = aspi->clk_freq;
> +	u32 hclk_div = 0; /* default value */
> +	u32 i;
> +	bool found = false;
> +
> +	/* FMC/SPIR10[11:8] */
> +	for (i = 0; i < ARRAY_SIZE(aspeed_spi_hclk_masks); i++) {
> +		if (ahb_freq / (i + 1) <= max_hz) {
> +			found = true;
> +			break;
> +		}
> +	}
> +
> +	if (found) {
> +		hclk_div = aspeed_spi_hclk_masks[i] << 8;
> +	} else {
> +		/* If FMC10[13] is set, an extra div_4 can be introduced. */
> +		for (i = 0; i < ARRAY_SIZE(aspeed_spi_hclk_masks); i++) {
> +			if (ahb_freq / ((i + 1) * 4) <= max_hz) {
> +				found = true;
> +				break;
> +			}
> +		}
> +
> +		if (found)
> +			hclk_div = BIT(13) | (aspeed_spi_hclk_masks[i] << 8);
> +	}
> +
> +	dev_dbg(aspi->dev, "found: %s, hclk: %d, max_clk: %d\n",
> +		found ? "yes" : "no", ahb_freq, max_hz);
> +
> +	if (found) {
> +		dev_dbg(aspi->dev, "h_div: %d (mask %x)\n",
> +			i + 1, aspeed_spi_hclk_masks[i]);
> +	}
> +
> +	return hclk_div;
> +}
> +
> +static u32 aspeed_spi_ast2600_clk_config(struct aspeed_spi_chip *chip,
> +					 u32 max_hz)
> +{
> +	struct aspeed_spi *aspi = chip->aspi;
> +	u32 ahb_freq = aspi->clk_freq;
> +	u32 hclk_div = 0x400; /* default value */
> +	u32 i, j;
> +	bool found = false;
> +
> +	/* FMC/SPIR10[27:24] */
> +	for (j = 0; j < 0xf; j++) {
> +		/* FMC/SPIR10[11:8] */
> +		for (i = 0; i < ARRAY_SIZE(aspeed_spi_hclk_masks); i++) {
> +			if (i == 0 && j == 0)
> +				continue;
> +
> +			if (ahb_freq / (i + 1 + (j * 16)) <= max_hz) {
> +				found = true;
> +				break;
> +			}
> +		}
> +
> +		if (found) {
> +			hclk_div = ((j << 24) | aspeed_spi_hclk_masks[i] << 8);
> +			break;
> +		}
> +	}
> +
> +	dev_dbg(aspi->dev, "found: %s, hclk: %d, max_clk: %d\n",
> +		found ? "yes" : "no", ahb_freq, max_hz);
> +
> +	if (found) {
> +		dev_dbg(aspi->dev, "base_clk: %d, h_div: %d (mask %x)\n",
> +			j, i + 1, aspeed_spi_hclk_masks[i]);
> +	}
> +
> +	return hclk_div;
> +}
> +
>   #define TIMING_DELAY_DI		BIT(3)
>   #define TIMING_DELAY_HCYCLE_MAX	5
>   #define TIMING_REG_AST2600(chip)				\
> @@ -1097,6 +1223,7 @@ static const struct aspeed_spi_data ast2400_fmc_data = {
>   	.segment_start = aspeed_spi_segment_start,
>   	.segment_end   = aspeed_spi_segment_end,
>   	.segment_reg   = aspeed_spi_segment_reg,
> +	.clk_config    = aspeed_spi_ast2400_clk_config,
>   };
>   
>   static const struct aspeed_spi_data ast2400_spi_data = {
> @@ -1109,6 +1236,7 @@ static const struct aspeed_spi_data ast2400_spi_data = {
>   	.hdiv_max      = 1,
>   	.calibrate     = aspeed_spi_calibrate,
>   	/* No segment registers */
> +	.clk_config    = aspeed_spi_ast2400_clk_config,
>   };
>   
>   static const struct aspeed_spi_data ast2500_fmc_data = {
> @@ -1117,12 +1245,13 @@ static const struct aspeed_spi_data ast2500_fmc_data = {
>   	.we0	       = 16,
>   	.ctl0	       = CE0_CTRL_REG,
>   	.timing	       = CE0_TIMING_COMPENSATION_REG,
> -	.hclk_mask     = 0xfffff0ff,
> +	.hclk_mask     = 0xffffd0ff,

I think this change of. hclk_mask and the one below should be included
in the initial patchset. I will when I publish v5.

Thanks,

C.

>   	.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,
> +	.clk_config    = aspeed_spi_ast2500_clk_config,
>   };
>   
>   static const struct aspeed_spi_data ast2500_spi_data = {
> @@ -1131,12 +1260,13 @@ static const struct aspeed_spi_data ast2500_spi_data = {
>   	.we0	       = 16,
>   	.ctl0	       = CE0_CTRL_REG,
>   	.timing	       = CE0_TIMING_COMPENSATION_REG,
> -	.hclk_mask     = 0xfffff0ff,
> +	.hclk_mask     = 0xffffd0ff,
>   	.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,
> +	.clk_config    = aspeed_spi_ast2500_clk_config,
>   };
>   
>   static const struct aspeed_spi_data ast2600_fmc_data = {
> @@ -1152,6 +1282,7 @@ static const struct aspeed_spi_data ast2600_fmc_data = {
>   	.segment_start = aspeed_spi_segment_ast2600_start,
>   	.segment_end   = aspeed_spi_segment_ast2600_end,
>   	.segment_reg   = aspeed_spi_segment_ast2600_reg,
> +	.clk_config    = aspeed_spi_ast2600_clk_config,
>   };
>   
>   static const struct aspeed_spi_data ast2600_spi_data = {
> @@ -1167,6 +1298,7 @@ static const struct aspeed_spi_data ast2600_spi_data = {
>   	.segment_start = aspeed_spi_segment_ast2600_start,
>   	.segment_end   = aspeed_spi_segment_ast2600_end,
>   	.segment_reg   = aspeed_spi_segment_ast2600_reg,
> +	.clk_config    = aspeed_spi_ast2600_clk_config,
>   };
>   
>   static const struct of_device_id aspeed_spi_matches[] = {



More information about the Linux-aspeed mailing list