From: "Cédric Le Goater" <clg@kaod.org>
To: Boris Brezillon <boris.brezillon@collabora.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
Vignesh Raghavendra <vigneshr@ti.com>,
linux-aspeed@lists.ozlabs.org,
Tudor Ambarus <tudor.ambarus@microchip.com>,
Andrew Jeffery <andrew@aj.id.au>,
Richard Weinberger <richard@nod.at>,
Marek Vasut <marek.vasut@gmail.com>,
linux-mtd@lists.infradead.org, Joel Stanley <joel@jms.id.au>,
Miquel Raynal <miquel.raynal@bootlin.com>,
Brian Norris <computersforpeace@gmail.com>,
David Woodhouse <dwmw2@infradead.org>,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 04/16] mtd: spi-nor: aspeed: Add read training
Date: Fri, 11 Oct 2019 15:55:25 +0200 [thread overview]
Message-ID: <3244b1ce-587c-6f12-cc9c-7eee0354e76b@kaod.org> (raw)
In-Reply-To: <20191011142805.6cc3905c@dhcp-172-31-174-146.wireless.concordia.ca>
On 11/10/2019 14:28, Boris Brezillon wrote:
> On Fri, 4 Oct 2019 13:59:07 +0200
> Cédric Le Goater <clg@kaod.org> wrote:
>
>> +#define ASPEED_SMC_HCLK_DIV(i) \
>> + (aspeed_smc_hclk_divs[(i) - 1] << CONTROL_CLOCK_FREQ_SEL_SHIFT)
>> +
>> +static u32 aspeed_smc_default_read(struct aspeed_smc_chip *chip)
>> +{
>> + /*
>> + * Keep the 4Byte address mode on the AST2400 SPI controller.
>> + * Other controllers set the 4Byte mode in the CE Control
>> + * Register
>> + */
>> + u32 ctl_mask = chip->controller->info == &spi_2400_info ?
>> + CONTROL_IO_ADDRESS_4B : 0;
>> +
>> + return (chip->ctl_val[smc_read] & ctl_mask) |
>> + (0x00 << 28) | /* Single bit */
>> + (0x00 << 24) | /* CE# max */
>> + (0x03 << 16) | /* use normal reads */
>> + (0x00 << 8) | /* HCLK/16 */
>> + (0x00 << 6) | /* no dummy cycle */
>> + (0x00); /* normal mode */
>
> IIUC, you're using a SPINOR_OP_READ operation to read the golden
> buffer, and if I'm right, you start reading at offset 0 of the dirmap
> window (offset 0 in the flash), so basically the first block in the NOR.
Yes.
> What happens if this block is erased? In that case your golden buf will
> contain only 0xff values, and the read calibration is likely to be
> useless
yes. that is why we have the aspeed_smc_check_calib_data() routine to
check that the data read makes some sense. If this is not the case, then :
"Calibration area too uniform, using low speed"
> (how can you determine if timings are good when IO pins always
> stay high). Don't we have a command that return non-ff/non-0 data while
> still being predictable and immutable?
Not that I know of on these controllers.
> Do you expect users to always
> flash a pattern that helps calibrating those delays?
This is the case on the OpenBMC systems, AFAICT.
u-boot.bin should be the data read on the FMC controller and the
SPI controller contains the host Firmware which is as random.
>
>> +}
>> +
>> +static int aspeed_smc_optimize_read(struct aspeed_smc_chip *chip,
>> + u32 max_freq)
>> +{
>> + u8 *golden_buf, *test_buf;
>> + int i, rc, best_div = -1;
>> + u32 save_read_val = chip->ctl_val[smc_read];
>> + u32 ahb_freq = chip->controller->clk_frequency;
>> +
>> + dev_dbg(chip->nor.dev, "AHB frequency: %d MHz", ahb_freq / 1000000);
>> +
>> + test_buf = kmalloc(CALIBRATE_BUF_SIZE * 2, GFP_KERNEL);
>> + golden_buf = test_buf + CALIBRATE_BUF_SIZE;
>> +
>> + /* We start with the dumbest setting (keep 4Byte bit) and read
>> + * some data
>> + */
>> + chip->ctl_val[smc_read] = aspeed_smc_default_read(chip);
>> +
>> + writel(chip->ctl_val[smc_read], chip->ctl);
>> +
>> + memcpy_fromio(golden_buf, chip->ahb_base, CALIBRATE_BUF_SIZE);
>> +
>> + /* Establish our read mode with freq field set to 0 (HCLK/16) */
>> + chip->ctl_val[smc_read] = save_read_val & 0xfffff0ff;
>> +
>> + /* Check if calibration data is suitable */
>> + if (!aspeed_smc_check_calib_data(golden_buf, CALIBRATE_BUF_SIZE)) {
>> + dev_info(chip->nor.dev,
>> + "Calibration area too uniform, using low speed");
>> + writel(chip->ctl_val[smc_read], chip->ctl);
>> + kfree(test_buf);
>> + return 0;
>> + }
>> +
>> + /* Now we iterate the HCLK dividers until we find our breaking point */
>> + for (i = ARRAY_SIZE(aspeed_smc_hclk_divs); i > 0; i--) {
>> + u32 tv, freq;
>> +
>> + /* Compare timing to max */
>> + freq = ahb_freq / i;
>> + if (freq > max_freq)
>> + continue;
>> +
>> + /* Set the timing */
>> + tv = chip->ctl_val[smc_read] | ASPEED_SMC_HCLK_DIV(i);
>> + writel(tv, chip->ctl);
>> + dev_dbg(chip->nor.dev, "Trying HCLK/%d...", i);
>> + rc = aspeed_smc_calibrate_reads(chip, i, golden_buf, test_buf);
>> + if (rc == 0)
>> + best_div = i;
>> + }
>> + kfree(test_buf);
>> +
>> + /* Nothing found ? */
>> + if (best_div < 0) {
>> + dev_warn(chip->nor.dev, "No good frequency, using dumb slow");
>> + } else {
>> + dev_dbg(chip->nor.dev, "Found good read timings at HCLK/%d",
>> + best_div);
>> + chip->ctl_val[smc_read] |= ASPEED_SMC_HCLK_DIV(best_div);
>> + }
>> +
>> + writel(chip->ctl_val[smc_read], chip->ctl);
>> + return 0;
>> +}
>
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
next prev parent reply other threads:[~2019-10-11 13:55 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-04 11:59 [PATCH 00/16] mtd: spi-nor: aspeed: AST2600 support and extensions Cédric Le Goater
2019-10-04 11:59 ` [PATCH 01/16] mtd: spi-nor: aspeed: Use command mode for reads Cédric Le Goater
2019-10-04 11:59 ` [PATCH 02/16] mtd: spi-nor: aspeed: Add support for SPI dual IO read mode Cédric Le Goater
2019-10-04 11:59 ` [PATCH 03/16] mtd: spi-nor: aspeed: Link controller with the ahb clock Cédric Le Goater
2019-10-04 11:59 ` [PATCH 04/16] mtd: spi-nor: aspeed: Add read training Cédric Le Goater
2019-10-11 12:28 ` Boris Brezillon
2019-10-11 13:13 ` Vignesh Raghavendra
2019-10-11 14:03 ` Cédric Le Goater
2019-10-11 13:55 ` Cédric Le Goater [this message]
2019-10-11 14:29 ` Boris Brezillon
2019-10-11 14:37 ` Cédric Le Goater
2019-10-04 11:59 ` [PATCH 05/16] mtd: spi-nor: aspeed: Limit the maximum SPI frequency Cédric Le Goater
2019-10-04 11:59 ` [PATCH 06/16] mtd: spi-nor: fix options for mx66l51235f Cédric Le Goater
2019-10-04 16:23 ` Cédric Le Goater
2019-10-04 11:59 ` [PATCH 07/16] mtd: spi-nor: aspeed: Add support for the 4B opcodes Cédric Le Goater
2019-10-04 11:59 ` [PATCH 08/16] mtd: spi-nor: Add support for w25q512jv Cédric Le Goater
2019-10-04 11:59 ` [PATCH 09/16] mtd: spi-nor: aspeed: Introduce a field for the AHB physical address Cédric Le Goater
2019-10-04 11:59 ` [PATCH 10/16] mtd: spi-nor: aspeed: Introduce segment operations Cédric Le Goater
2019-10-04 11:59 ` [PATCH 11/16] dt-bindings: mtd: aspeed-smc: Add new comptatible for AST2600 Cédric Le Goater
2019-10-15 19:26 ` Rob Herring
2019-10-04 11:59 ` [PATCH 12/16] mtd: spi-nor: aspeed: Add initial support for the AST2600 Cédric Le Goater
2019-10-04 11:59 ` [PATCH 13/16] mtd: spi-nor: aspeed: Check for disabled segments on " Cédric Le Goater
2019-10-04 12:09 ` [PATCH 14/16] mtd: spi-nor: aspeed: Introduce training operations per platform Cédric Le Goater
2019-10-04 12:09 ` [PATCH 15/16] mtd: spi-nor: aspeed: Introduce a HCLK mask for training Cédric Le Goater
2019-10-04 12:09 ` [PATCH 16/16] mtd: spi-nor: aspeed: Add read training support for the AST2600 Cédric Le Goater
2019-10-09 20:55 ` [PATCH 00/16] mtd: spi-nor: aspeed: AST2600 support and extensions Boris Brezillon
2019-10-10 23:47 ` Joel Stanley
2019-10-11 6:45 ` Boris Brezillon
2019-10-11 9:29 ` Cédric Le Goater
2019-10-11 9:51 ` Boris Brezillon
2019-10-11 11:47 ` Cédric Le Goater
2019-10-11 12:07 ` Boris Brezillon
2019-10-11 13:07 ` Cédric Le Goater
2019-10-11 14:01 ` Boris Brezillon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3244b1ce-587c-6f12-cc9c-7eee0354e76b@kaod.org \
--to=clg@kaod.org \
--cc=andrew@aj.id.au \
--cc=boris.brezillon@collabora.com \
--cc=computersforpeace@gmail.com \
--cc=dwmw2@infradead.org \
--cc=joel@jms.id.au \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-aspeed@lists.ozlabs.org \
--cc=linux-mtd@lists.infradead.org \
--cc=marek.vasut@gmail.com \
--cc=mark.rutland@arm.com \
--cc=miquel.raynal@bootlin.com \
--cc=richard@nod.at \
--cc=tudor.ambarus@microchip.com \
--cc=vigneshr@ti.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).