[PATCH 1/1] spi: aspeed: Update SPI clock frequency configuration
Chin-Ting Kuo
chin-ting_kuo at aspeedtech.com
Tue Apr 19 12:45:08 AEST 2022
Hi Cédric,
> -----Original Message-----
> From: Cédric Le Goater <clg at kaod.org>
> Sent: Tuesday, April 19, 2022 4:42 AM
> To: Chin-Ting Kuo <chin-ting_kuo at aspeedtech.com>;
> linux-aspeed at lists.ozlabs.org; openbmc at lists.ozlabs.org
> Subject: Re: [PATCH 1/1] spi: aspeed: Update SPI clock frequency configuration
>
> 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"
Okay.
>
> > - 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[]
>
Yes, I will try to merge this array into 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.
>
Okay.
Thanks.
Chin-Ting
> 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 openbmc
mailing list