[PATCH linux dev-4.10] mtd: spi-nor: aspeed: limit the maximum SPI frequency
Cédric Le Goater
clg at kaod.org
Mon Jul 3 16:57:48 AEST 2017
On 07/03/2017 05:00 AM, Joel Stanley wrote:
> On Mon, Jul 3, 2017 at 7:19 AM, Cédric Le Goater <clg at kaod.org> wrote:
>> The optimize read algo can choose a 100MHz SPI frequency which might
>> be a bit too high for dual output IO on some chips, for the W25Q256 on
>> palmetto for instance. The MX66L1G45G on witherspoon should be fine
>> though.
>>
>> To do some fine tuning, we use the "spi-max-frequency" property in the
>> device tree. By default, the frequency is 50MHz, as U-boot does for
>> most of the supported chips.
>
> Looks good to me. I had some questions below to clear up my understanding.
>
> I will boot this on a few machines today for you.
ok. I tried it on a palmetto, a ast2500 evb and a witherspoon.
> Is this a suspect for the first-byte corruption issue?
yes. This is still a mystery for me. I have hammered pretty
badly the PNOR on a palmetto and on a witherspoon and could
not reproduce ...
50MHz seems to be the safe limit used by most of the tool I
have looked at. I will ask a couple of questions to Aspeed.
> Cheers,
>
> Joel
>
>>
>> Signed-off-by: Cédric Le Goater <clg at kaod.org>
>> ---
>> drivers/mtd/spi-nor/aspeed-smc.c | 69 +++++++++++++++++++++++++---------------
>> 1 file changed, 44 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/mtd/spi-nor/aspeed-smc.c b/drivers/mtd/spi-nor/aspeed-smc.c
>> index 23622bf62b6b..7d9f570f11ff 100644
>> --- a/drivers/mtd/spi-nor/aspeed-smc.c
>> +++ b/drivers/mtd/spi-nor/aspeed-smc.c
>> @@ -121,6 +121,7 @@ struct aspeed_smc_chip {
>> u32 ctl_val[smc_max]; /* control settings */
>> enum aspeed_smc_flash_type type; /* what type of flash */
>> struct spi_nor nor;
>> + u32 clk_rate;
>> };
>>
>> struct aspeed_smc_controller {
>> @@ -148,6 +149,8 @@ struct aspeed_smc_controller {
>> struct aspeed_smc_chip *chips[0]; /* pointers to attached chips */
>> };
>>
>> +#define ASPEED_SPI_DEFAULT_FREQ 50000000
>> +
>> /*
>> * SPI Flash Configuration Register (AST2500 SPI)
>> * or
>> @@ -215,7 +218,7 @@ struct aspeed_smc_controller {
>>
>> #define CONTROL_KEEP_MASK \
>> (CONTROL_AAF_MODE | CONTROL_CE_INACTIVE_MASK | CONTROL_CLK_DIV4 | \
>> - CONTROL_CLOCK_FREQ_SEL_MASK | CONTROL_LSB_FIRST | CONTROL_CLOCK_MODE_3)
>> + CONTROL_LSB_FIRST | CONTROL_CLOCK_MODE_3)
>
> We're dropping CONTROL_CLOCK_FREQ_SEL_MASK because the driver now sets
> it own value?
Exactly.
The BMC freq is tuned by U-Boot at HCLK/4 (~50MHz) and the
PNOR is set with a default HCLK/2 (~100MHz) which is done
by HW strapping I suppose.
It is better to enforce some settings I think.
>> /* Interrupt Control and Status Register */
>> #define INTERRUPT_STATUS_REG 0x08
>> @@ -820,6 +823,34 @@ static void aspeed_smc_chip_set_4b_spi_2400(struct aspeed_smc_chip *chip)
>> chip->ctl_val[smc_read] |= CONTROL_IO_ADDRESS_4B;
>> }
>>
>> +static const u32 aspeed_smc_hclk_divs[] = {
>
> Could be a u8.
Indeed. I will change that.
>> + 0xf, /* HCLK */
>> + 0x7, /* HCLK/2 */
>> + 0xe, /* HCLK/3 */
>> + 0x6, /* HCLK/4 */
>> + 0xd, /* HCLK/5 */
>> +};
>
> You stop at HCLK/5 as there's no point going any slower?
The first reason is that the "SPI Flash Read Timing Setting"
register only handles this range, but we could change the
optimize algo to stop or start at HCLK/5 and have a bigger
array of dividers.
The second is that all the chips we use support freqs above
40MHz, which is more or less HCLK/5.
There is some room for improvements. This is not a problem,
I can change that.
Thanks,
C.
>> +
>> +#define ASPEED_SMC_HCLK_DIV(i) (aspeed_smc_hclk_divs[(i) - 1] << 8)
>> +
>> +static u32 aspeed_smc_get_hclk(struct aspeed_smc_chip *chip,
>> + u32 max_freq)
>> +{
>> + int i;
>> + u32 div;
>> + u32 ahb_freq = clk_get_rate(chip->controller->ahb_clk);
>> +
>> + for (i = ARRAY_SIZE(aspeed_smc_hclk_divs); i > 0; i--) {
>> + u32 freq = ahb_freq / i;
>> +
>> + if (freq >= max_freq)
>> + break;
>> +
>> + div = ASPEED_SMC_HCLK_DIV(i);
>> + }
>> + return div;
>> +}
>> +
>> static int aspeed_smc_chip_setup_init(struct aspeed_smc_chip *chip,
>> struct resource *res)
>> {
>> @@ -858,6 +889,7 @@ static int aspeed_smc_chip_setup_init(struct aspeed_smc_chip *chip,
>> dev_dbg(controller->dev, "control register: %08x\n", reg);
>>
>> base_reg = reg & CONTROL_KEEP_MASK;
>> + base_reg |= aspeed_smc_get_hclk(chip, chip->clk_rate);
>> if (base_reg != reg) {
>> dev_dbg(controller->dev,
>> "control register changed to: %08x\n",
>> @@ -865,17 +897,9 @@ static int aspeed_smc_chip_setup_init(struct aspeed_smc_chip *chip,
>> }
>> chip->ctl_val[smc_base] = base_reg;
>>
>> - /*
>> - * Retain the prior value of the control register as the
>> - * default if it was normal access mode. Otherwise start with
>> - * the sanitized base value set to read mode.
>> - */
>> - if ((reg & CONTROL_COMMAND_MODE_MASK) ==
>> - CONTROL_COMMAND_MODE_NORMAL)
>> - chip->ctl_val[smc_read] = reg;
>> - else
>> - chip->ctl_val[smc_read] = chip->ctl_val[smc_base] |
>> - CONTROL_COMMAND_MODE_NORMAL;
>> + /* start with the sanitized base value set to read mode. */
>> + chip->ctl_val[smc_read] = chip->ctl_val[smc_base] |
>> + CONTROL_COMMAND_MODE_NORMAL;
>>
>> dev_dbg(controller->dev, "default control register: %08x\n",
>> chip->ctl_val[smc_read]);
>> @@ -968,15 +992,6 @@ static bool aspeed_smc_check_calib_data(const u8 *test_buf, u32 size)
>> return cnt >= 64;
>> }
>>
>> -static const uint32_t aspeed_smc_hclk_divs[] = {
>> - 0xf, /* HCLK */
>> - 0x7, /* HCLK/2 */
>> - 0xe, /* HCLK/3 */
>> - 0x6, /* HCLK/4 */
>> - 0xd, /* HCLK/5 */
>> -};
>> -#define ASPEED_SMC_HCLK_DIV(i) (aspeed_smc_hclk_divs[(i) - 1] << 8)
>> -
>> static int aspeed_smc_optimize_read(struct aspeed_smc_chip *chip,
>> u32 max_freq)
>> {
>> @@ -1104,11 +1119,8 @@ static int aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip)
>> dev_dbg(controller->dev, "read control register: %08x\n",
>> chip->ctl_val[smc_read]);
>>
>> - /*
>> - * TODO: get max freq from chip
>> - */
>> if (optimize_read && info->optimize_read)
>> - info->optimize_read(chip, 104000000);
>> + info->optimize_read(chip, chip->clk_rate);
>> return 0;
>> }
>>
>> @@ -1156,6 +1168,13 @@ static int aspeed_smc_setup_flash(struct aspeed_smc_controller *controller,
>> break;
>> }
>>
>> + if (of_property_read_u32(child, "spi-max-frequency",
>> + &chip->clk_rate)) {
>> + chip->clk_rate = ASPEED_SPI_DEFAULT_FREQ;
>> + }
>> + dev_info(dev, "Using %d MHz SPI frequency\n",
>> + chip->clk_rate / 1000000);
>> +
>> chip->controller = controller;
>> chip->ctl = controller->regs + info->ctl0 + cs * 4;
>> chip->cs = cs;
>> --
>> 2.7.5
>>
More information about the openbmc
mailing list