Hi,
This series adds a new SPI driver using the spi-mem interface for the
Aspeed static memory controllers of the AST2600, AST2500 and AST2400
SoCs.
* AST2600 Firmware SPI Memory Controller (FMC)
* AST2600 SPI Flash Controller (SPI1 and SPI2)
* AST2500 Firmware SPI Memory Controller (FMC)
* AST2500 SPI Flash Controller (SPI1 and SPI2)
* AST2400 New Static Memory Controller (also referred as FMC)
* AST2400 SPI Flash Controller (SPI)
It is based on the current OpenBMC kernel driver [1], using directly
the MTD SPI-NOR interface and on a patchset [2] previously proposed
adding support for the AST2600 only. This driver takes a slightly
different approach to cover all 6 controllers.
It does not make use of the controller register disabling Address and
Data byte lanes because is not available on the AST2400 SoC. We could
introduce a specific handler for new features available on recent SoCs
if needed. As there is not much difference on performance, the driver
chooses the common denominator: "User mode" which has been heavily
tested in [1]. "User mode" is also used as a fall back method when
flash device mapping window is too small.
Problems to address with spi-mem were the configuration of the mapping
windows and the calibration of the read timings. The driver handles
them in the direct mapping handler when some knowledge on the size of
the flash device is know. It is not perfect but not incorrect either.
The algorithm is one from [1] because it doesn't require the DMA
registers which are not available on all controllers.
Direct mapping for writes is not supported (yet). I have seen some
corruption with writes and I preferred to use the safer and proven
method of the initial driver [1]. We can improve that later.
The driver supports Quad SPI RX transfers on the AST2600 SoC but it
didn't have the expected results. Therefore it is not activated yet.
This needs more tests.
The series does not remove the current Aspeed SMC driver but prepares
ground for its removal by changing its CONFIG option. This last step
can be addressed as a followup when the new driver using the spi-mem
interface has been sufficiently exposed.
Tested on:
* OpenPOWER Palmetto (AST2400)
* Evaluation board (AST2500)
* OpenPOWER Witherspoon (AST2500)
* Evaluation board (AST2600 A0)
* Rainier board (AST2600)
[1] https://github.com/openbmc/linux/blob/dev-5.15/drivers/mtd/spi-nor/controllers/aspeed-smc.c
[2] https://patchwork.ozlabs.org/project/linux-aspeed/list/?series=212394
Thanks,
C.
Cédric Le Goater (10):
mtd: spi-nor: aspeed: Rename Kconfig option
dt-bindings: spi: Add Aspeed SMC controllers device tree binding
spi: spi-mem: Add driver for Aspeed SMC controllers
spi: aspeed: Add support for direct mapping
spi: aspeed: Adjust direct mapping to device size
spi: aspeed: Workaround AST2500 limitations
spi: aspeed: Add support for the AST2400 SPI controller
spi: aspeed: Calibrate read timings
ARM: dts: aspeed: Enable Dual SPI RX transfers
spi: aspeed: Activate new spi-mem driver
drivers/spi/spi-aspeed-smc.c | 1241 +++++++++++++++++
.../bindings/spi/aspeed,ast2600-fmc.yaml | 92 ++
arch/arm/boot/dts/aspeed-g4.dtsi | 6 +
arch/arm/boot/dts/aspeed-g5.dtsi | 7 +
arch/arm/boot/dts/aspeed-g6.dtsi | 8 +
drivers/mtd/spi-nor/controllers/Kconfig | 4 +-
drivers/mtd/spi-nor/controllers/Makefile | 2 +-
drivers/spi/Kconfig | 11 +
drivers/spi/Makefile | 1 +
9 files changed, 1369 insertions(+), 3 deletions(-)
create mode 100644 drivers/spi/spi-aspeed-smc.c
create mode 100644 Documentation/devicetree/bindings/spi/aspeed,ast2600-fmc.yaml
--
2.34.1
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 a bit 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 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.
Due to the lack of API, calibration is performed when the direct mapping
for reads is created.
Cc: Pratyush Yadav <[email protected]>
Signed-off-by: Cédric Le Goater <[email protected]>
---
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 e44e80bab50f..a08c20308404 100644
--- a/drivers/spi/spi-aspeed-smc.c
+++ b/drivers/spi/spi-aspeed-smc.c
@@ -35,6 +35,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
@@ -47,6 +49,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,
@@ -72,10 +77,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
@@ -540,6 +550,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);
@@ -588,6 +600,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]);
@@ -869,6 +883,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:
+ 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
*/
@@ -877,6 +1134,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,
@@ -887,6 +1148,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 */
};
@@ -895,6 +1160,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,
@@ -905,6 +1174,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,
@@ -916,6 +1189,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,
@@ -927,6 +1204,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
Extend the driver for the AST2400 SPI Flash Controller (SPI). This
controller has a slightly different interface which requires
adaptation of the 4B handling. Summary of features :
. host Firmware
. 1 chip select pin (CE0)
. slightly different register set, between AST2500 and the legacy
controller
. no segment registers
. single, dual mode.
Signed-off-by: Cédric Le Goater <[email protected]>
---
drivers/spi/spi-aspeed-smc.c | 33 ++++++++++++++++++++++++++++++++-
1 file changed, 32 insertions(+), 1 deletion(-)
diff --git a/drivers/spi/spi-aspeed-smc.c b/drivers/spi/spi-aspeed-smc.c
index c6ac3253d7d5..e44e80bab50f 100644
--- a/drivers/spi/spi-aspeed-smc.c
+++ b/drivers/spi/spi-aspeed-smc.c
@@ -32,6 +32,7 @@
#define CTRL_IO_DUAL_DATA BIT(29)
#define CTRL_IO_QUAD_DATA BIT(30)
#define CTRL_COMMAND_SHIFT 16
+#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_CE_STOP_ACTIVE BIT(2)
@@ -293,6 +294,8 @@ static bool aspeed_spi_supports_op(struct spi_mem *mem, const struct spi_mem_op
return true;
}
+static const struct aspeed_spi_data ast2400_spi_data;
+
static int do_aspeed_spi_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
{
struct aspeed_spi *aspi = spi_controller_get_devdata(mem->spi->master);
@@ -322,6 +325,9 @@ static int do_aspeed_spi_exec_op(struct spi_mem *mem, const struct spi_mem_op *o
addr_mode |= (0x11 << chip->cs);
else
addr_mode &= ~(0x11 << chip->cs);
+
+ if (op->addr.nbytes == 4 && chip->aspi->data == &ast2400_spi_data)
+ ctl_val |= CTRL_IO_ADDRESS_4B;
}
if (op->dummy.buswidth && op->dummy.nbytes)
@@ -413,7 +419,13 @@ static int aspeed_spi_chip_set_default_window(struct aspeed_spi_chip *chip)
struct aspeed_spi_window windows[ASPEED_SPI_MAX_NUM_CS] = { 0 };
struct aspeed_spi_window *win = &windows[chip->cs];
- aspeed_spi_get_windows(aspi, windows);
+ /* No segment registers for the AST2400 SPI controller */
+ if (aspi->data == &ast2400_spi_data) {
+ win->offset = 0;
+ win->size = aspi->ahb_window_size;
+ } else {
+ aspeed_spi_get_windows(aspi, windows);
+ }
chip->ahb_base = aspi->ahb_base + win->offset;
chip->ahb_window_size = win->size;
@@ -476,6 +488,10 @@ static int aspeed_spi_chip_adjust_window(struct aspeed_spi_chip *chip,
struct aspeed_spi_window *win = &windows[chip->cs];
int ret;
+ /* No segment registers for the AST2400 SPI controller */
+ if (aspi->data == &ast2400_spi_data)
+ return 0;
+
/*
* Due to an HW issue on the AST2500 SPI controller, the CE0
* window size should be smaller than the maximum 128MB.
@@ -560,6 +576,12 @@ static int aspeed_spi_dirmap_create(struct spi_mem_dirmap_desc *desc)
else
addr_mode &= ~(0x11 << chip->cs);
writel(addr_mode, aspi->regs + CE_CTRL_REG);
+
+ /* AST2400 SPI controller sets 4BYTE address mode in
+ * CE0 Control Register
+ */
+ if (op->addr.nbytes == 4 && chip->aspi->data == &ast2400_spi_data)
+ ctl_val |= CTRL_IO_ADDRESS_4B;
}
/* READ mode is the controller default setting */
@@ -860,6 +882,14 @@ static const struct aspeed_spi_data ast2400_fmc_data = {
.segment_reg = aspeed_spi_segment_reg,
};
+static const struct aspeed_spi_data ast2400_spi_data = {
+ .max_cs = 1,
+ .hastype = false,
+ .we0 = 0,
+ .ctl0 = 0x04,
+ /* No segment registers */
+};
+
static const struct aspeed_spi_data ast2500_fmc_data = {
.max_cs = 3,
.hastype = true,
@@ -904,6 +934,7 @@ static const struct aspeed_spi_data ast2600_spi_data = {
static const struct of_device_id aspeed_spi_matches[] = {
{ .compatible = "aspeed,ast2400-fmc", .data = &ast2400_fmc_data },
+ { .compatible = "aspeed,ast2400-spi", .data = &ast2400_spi_data },
{ .compatible = "aspeed,ast2500-fmc", .data = &ast2500_fmc_data },
{ .compatible = "aspeed,ast2500-spi", .data = &ast2500_spi_data },
{ .compatible = "aspeed,ast2600-fmc", .data = &ast2600_fmc_data },
--
2.34.1
The "interrupt" property is optional because it is only necessary for
controllers supporting DMAs (Not implemented yet in the new driver).
Cc: Chin-Ting Kuo <[email protected]>
Signed-off-by: Cédric Le Goater <[email protected]>
---
.../bindings/spi/aspeed,ast2600-fmc.yaml | 92 +++++++++++++++++++
1 file changed, 92 insertions(+)
create mode 100644 Documentation/devicetree/bindings/spi/aspeed,ast2600-fmc.yaml
diff --git a/Documentation/devicetree/bindings/spi/aspeed,ast2600-fmc.yaml b/Documentation/devicetree/bindings/spi/aspeed,ast2600-fmc.yaml
new file mode 100644
index 000000000000..ed71c4d86930
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/aspeed,ast2600-fmc.yaml
@@ -0,0 +1,92 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/spi/aspeed,ast2600-fmc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Aspeed SMC controllers bindings
+
+maintainers:
+ - Chin-Ting Kuo <[email protected]>
+ - Cédric Le Goater <[email protected]>
+
+description: |
+ This binding describes the Aspeed Static Memory Controllers (FMC and
+ SPI) of the AST2400, AST2500 and AST2600 SOCs.
+
+allOf:
+ - $ref: "spi-controller.yaml#"
+
+properties:
+ compatible:
+ enum:
+ - aspeed,ast2600-fmc
+ - aspeed,ast2600-spi
+ - aspeed,ast2500-fmc
+ - aspeed,ast2500-spi
+ - aspeed,ast2400-fmc
+ - aspeed,ast2400-spi
+
+ reg:
+ items:
+ - description: registers
+ - description: memory mapping
+
+ clocks:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+patternProperties:
+ "@[0-9a-f]+":
+ type: object
+
+ properties:
+ spi-rx-bus-width:
+ enum: [1, 2, 4]
+
+ required:
+ - reg
+
+required:
+ - compatible
+ - reg
+ - clocks
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/interrupt-controller/aspeed-scu-ic.h>
+ #include <dt-bindings/clock/ast2600-clock.h>
+
+ spi@1e620000 {
+ reg = < 0x1e620000 0xc4
+ 0x20000000 0x10000000 >;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "aspeed,ast2600-fmc";
+ clocks = <&syscon ASPEED_CLK_AHB>;
+ status = "disabled";
+ interrupts = <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>;
+ flash@0 {
+ reg = < 0 >;
+ compatible = "jedec,spi-nor";
+ spi-max-frequency = <50000000>;
+ status = "disabled";
+ };
+ flash@1 {
+ reg = < 1 >;
+ compatible = "jedec,spi-nor";
+ spi-max-frequency = <50000000>;
+ status = "disabled";
+ };
+ flash@2 {
+ reg = < 2 >;
+ compatible = "jedec,spi-nor";
+ spi-max-frequency = <50000000>;
+ status = "disabled";
+ };
+ };
--
2.34.1
The previous driver using the MTD SPI NOR interface is kept in case we
find some issues but we should remove it quickly once the new driver
using the spi-mem interface has been sufficiently exposed.
Signed-off-by: Cédric Le Goater <[email protected]>
---
arch/arm/configs/aspeed_g4_defconfig | 2 +-
arch/arm/configs/aspeed_g5_defconfig | 2 +-
arch/arm/configs/multi_v5_defconfig | 2 +-
arch/arm/configs/multi_v7_defconfig | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm/configs/aspeed_g4_defconfig b/arch/arm/configs/aspeed_g4_defconfig
index 964536444cd7..b4a1b2ed1a17 100644
--- a/arch/arm/configs/aspeed_g4_defconfig
+++ b/arch/arm/configs/aspeed_g4_defconfig
@@ -64,7 +64,7 @@ CONFIG_MTD_BLOCK=y
CONFIG_MTD_PARTITIONED_MASTER=y
CONFIG_MTD_SPI_NOR=y
# CONFIG_MTD_SPI_NOR_USE_4K_SECTORS is not set
-CONFIG_SPI_ASPEED_SMC_MTD_SPI_NOR=y
+CONFIG_SPI_ASPEED_SMC=y
CONFIG_MTD_UBI=y
CONFIG_MTD_UBI_FASTMAP=y
CONFIG_MTD_UBI_BLOCK=y
diff --git a/arch/arm/configs/aspeed_g5_defconfig b/arch/arm/configs/aspeed_g5_defconfig
index e809236ca88b..ccc4240ee4b5 100644
--- a/arch/arm/configs/aspeed_g5_defconfig
+++ b/arch/arm/configs/aspeed_g5_defconfig
@@ -103,7 +103,7 @@ CONFIG_MTD_BLOCK=y
CONFIG_MTD_PARTITIONED_MASTER=y
CONFIG_MTD_SPI_NOR=y
# CONFIG_MTD_SPI_NOR_USE_4K_SECTORS is not set
-CONFIG_SPI_ASPEED_SMC_MTD_SPI_NOR=y
+CONFIG_SPI_ASPEED_SMC=y
CONFIG_MTD_UBI=y
CONFIG_MTD_UBI_FASTMAP=y
CONFIG_MTD_UBI_BLOCK=y
diff --git a/arch/arm/configs/multi_v5_defconfig b/arch/arm/configs/multi_v5_defconfig
index 49083ef05fb0..80a3ae02d759 100644
--- a/arch/arm/configs/multi_v5_defconfig
+++ b/arch/arm/configs/multi_v5_defconfig
@@ -103,7 +103,7 @@ CONFIG_MTD_RAW_NAND=y
CONFIG_MTD_NAND_ATMEL=y
CONFIG_MTD_NAND_ORION=y
CONFIG_MTD_SPI_NOR=y
-CONFIG_SPI_ASPEED_SMC_MTD_SPI_NOR=y
+CONFIG_SPI_ASPEED_SMC=y
CONFIG_MTD_UBI=y
CONFIG_BLK_DEV_LOOP=y
CONFIG_ATMEL_SSC=m
diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index fc1b69256b64..33572998dbbe 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -217,7 +217,7 @@ CONFIG_MTD_NAND_DAVINCI=y
CONFIG_MTD_NAND_STM32_FMC2=y
CONFIG_MTD_NAND_PL35X=y
CONFIG_MTD_SPI_NOR=y
-CONFIG_SPI_ASPEED_SMC_MTD_SPI_NOR=m
+CONFIG_SPI_ASPEED_SMC=m
CONFIG_MTD_UBI=y
CONFIG_BLK_DEV_LOOP=y
CONFIG_BLK_DEV_RAM=y
--
2.34.1
It is not possible to configure a full 128MB window for a chip of the
same size on the AST2500 SPI controller. For his case, the maximum
window size is restricted to 120MB for CE0.
Signed-off-by: Cédric Le Goater <[email protected]>
---
drivers/spi/spi-aspeed-smc.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/spi/spi-aspeed-smc.c b/drivers/spi/spi-aspeed-smc.c
index 9c4eacfed72c..c6ac3253d7d5 100644
--- a/drivers/spi/spi-aspeed-smc.c
+++ b/drivers/spi/spi-aspeed-smc.c
@@ -466,6 +466,8 @@ static int aspeed_spi_set_window(struct aspeed_spi *aspi,
* - ioremap each window, not strictly necessary since the overall window
* is correct.
*/
+static const struct aspeed_spi_data ast2500_spi_data;
+
static int aspeed_spi_chip_adjust_window(struct aspeed_spi_chip *chip,
u32 local_offset, u32 size)
{
@@ -474,6 +476,16 @@ static int aspeed_spi_chip_adjust_window(struct aspeed_spi_chip *chip,
struct aspeed_spi_window *win = &windows[chip->cs];
int ret;
+ /*
+ * Due to an HW issue on the AST2500 SPI controller, the CE0
+ * window size should be smaller than the maximum 128MB.
+ */
+ if (aspi->data == &ast2500_spi_data && chip->cs == 0 && size == SZ_128M) {
+ size = 120 << 20;
+ dev_info(aspi->dev, "CE%d window resized to %dMB (AST2500 HW quirk)",
+ chip->cs, size >> 20);
+ }
+
aspeed_spi_get_windows(aspi, windows);
/* Adjust this chip window */
--
2.34.1
On Mon, 14 Feb 2022 10:42:23 +0100, Cédric Le Goater wrote:
> The "interrupt" property is optional because it is only necessary for
> controllers supporting DMAs (Not implemented yet in the new driver).
>
> Cc: Chin-Ting Kuo <[email protected]>
> Signed-off-by: Cédric Le Goater <[email protected]>
> ---
> .../bindings/spi/aspeed,ast2600-fmc.yaml | 92 +++++++++++++++++++
> 1 file changed, 92 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/spi/aspeed,ast2600-fmc.yaml
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/spi/aspeed,ast2600-fmc.example.dt.yaml: example-0: spi@1e620000:reg:0: [509739008, 196, 536870912, 268435456] is too long
From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/spi/aspeed,ast2600-fmc.example.dt.yaml: spi@1e620000: reg: [[509739008, 196, 536870912, 268435456]] is too short
From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/spi/aspeed,ast2600-fmc.yaml
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/patch/1592369
This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit.
On Mon, Feb 14, 2022 at 10:42:23AM +0100, C?dric Le Goater wrote:
> The "interrupt" property is optional because it is only necessary for
> controllers supporting DMAs (Not implemented yet in the new driver).
>
> Cc: Chin-Ting Kuo <[email protected]>
> Signed-off-by: C?dric Le Goater <[email protected]>
> ---
> .../bindings/spi/aspeed,ast2600-fmc.yaml | 92 +++++++++++++++++++
> 1 file changed, 92 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/spi/aspeed,ast2600-fmc.yaml
>
> diff --git a/Documentation/devicetree/bindings/spi/aspeed,ast2600-fmc.yaml b/Documentation/devicetree/bindings/spi/aspeed,ast2600-fmc.yaml
> new file mode 100644
> index 000000000000..ed71c4d86930
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/aspeed,ast2600-fmc.yaml
> @@ -0,0 +1,92 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/spi/aspeed,ast2600-fmc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Aspeed SMC controllers bindings
> +
> +maintainers:
> + - Chin-Ting Kuo <[email protected]>
> + - C?dric Le Goater <[email protected]>
> +
> +description: |
> + This binding describes the Aspeed Static Memory Controllers (FMC and
> + SPI) of the AST2400, AST2500 and AST2600 SOCs.
> +
> +allOf:
> + - $ref: "spi-controller.yaml#"
> +
> +properties:
> + compatible:
> + enum:
> + - aspeed,ast2600-fmc
> + - aspeed,ast2600-spi
> + - aspeed,ast2500-fmc
> + - aspeed,ast2500-spi
> + - aspeed,ast2400-fmc
> + - aspeed,ast2400-spi
> +
> + reg:
> + items:
> + - description: registers
> + - description: memory mapping
> +
> + clocks:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> +patternProperties:
> + "@[0-9a-f]+":
> + type: object
> +
> + properties:
> + spi-rx-bus-width:
> + enum: [1, 2, 4]
> +
> + required:
> + - reg
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/interrupt-controller/aspeed-scu-ic.h>
> + #include <dt-bindings/clock/ast2600-clock.h>
> +
> + spi@1e620000 {
> + reg = < 0x1e620000 0xc4
> + 0x20000000 0x10000000 >;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "aspeed,ast2600-fmc";
> + clocks = <&syscon ASPEED_CLK_AHB>;
> + status = "disabled";
Why is your example disabled? Drop 'status'.
> + interrupts = <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>;
> + flash@0 {
> + reg = < 0 >;
> + compatible = "jedec,spi-nor";
> + spi-max-frequency = <50000000>;
> + status = "disabled";
Ditto.
> + };
> + flash@1 {
> + reg = < 1 >;
> + compatible = "jedec,spi-nor";
> + spi-max-frequency = <50000000>;
> + status = "disabled";
> + };
> + flash@2 {
> + reg = < 2 >;
> + compatible = "jedec,spi-nor";
> + spi-max-frequency = <50000000>;
> + status = "disabled";
> + };
> + };
> --
> 2.34.1
>
>
On Mon, 14 Feb 2022 at 09:43, Cédric Le Goater <[email protected]> wrote:
>
> The previous driver using the MTD SPI NOR interface is kept in case we
> find some issues but we should remove it quickly once the new driver
> using the spi-mem interface has been sufficiently exposed.
>
> Signed-off-by: Cédric Le Goater <[email protected]>
I suggest we drop the defconfig changes from both this patch and the
first. This way we'll always have the new driver being built, with
less churn.
If you strongly prefer the way you've done it then that's fine too.
> ---
> arch/arm/configs/aspeed_g4_defconfig | 2 +-
> arch/arm/configs/aspeed_g5_defconfig | 2 +-
> arch/arm/configs/multi_v5_defconfig | 2 +-
> arch/arm/configs/multi_v7_defconfig | 2 +-
> 4 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/configs/aspeed_g4_defconfig b/arch/arm/configs/aspeed_g4_defconfig
> index 964536444cd7..b4a1b2ed1a17 100644
> --- a/arch/arm/configs/aspeed_g4_defconfig
> +++ b/arch/arm/configs/aspeed_g4_defconfig
> @@ -64,7 +64,7 @@ CONFIG_MTD_BLOCK=y
> CONFIG_MTD_PARTITIONED_MASTER=y
> CONFIG_MTD_SPI_NOR=y
> # CONFIG_MTD_SPI_NOR_USE_4K_SECTORS is not set
> -CONFIG_SPI_ASPEED_SMC_MTD_SPI_NOR=y
> +CONFIG_SPI_ASPEED_SMC=y
> CONFIG_MTD_UBI=y
> CONFIG_MTD_UBI_FASTMAP=y
> CONFIG_MTD_UBI_BLOCK=y
> diff --git a/arch/arm/configs/aspeed_g5_defconfig b/arch/arm/configs/aspeed_g5_defconfig
> index e809236ca88b..ccc4240ee4b5 100644
> --- a/arch/arm/configs/aspeed_g5_defconfig
> +++ b/arch/arm/configs/aspeed_g5_defconfig
> @@ -103,7 +103,7 @@ CONFIG_MTD_BLOCK=y
> CONFIG_MTD_PARTITIONED_MASTER=y
> CONFIG_MTD_SPI_NOR=y
> # CONFIG_MTD_SPI_NOR_USE_4K_SECTORS is not set
> -CONFIG_SPI_ASPEED_SMC_MTD_SPI_NOR=y
> +CONFIG_SPI_ASPEED_SMC=y
> CONFIG_MTD_UBI=y
> CONFIG_MTD_UBI_FASTMAP=y
> CONFIG_MTD_UBI_BLOCK=y
> diff --git a/arch/arm/configs/multi_v5_defconfig b/arch/arm/configs/multi_v5_defconfig
> index 49083ef05fb0..80a3ae02d759 100644
> --- a/arch/arm/configs/multi_v5_defconfig
> +++ b/arch/arm/configs/multi_v5_defconfig
> @@ -103,7 +103,7 @@ CONFIG_MTD_RAW_NAND=y
> CONFIG_MTD_NAND_ATMEL=y
> CONFIG_MTD_NAND_ORION=y
> CONFIG_MTD_SPI_NOR=y
> -CONFIG_SPI_ASPEED_SMC_MTD_SPI_NOR=y
> +CONFIG_SPI_ASPEED_SMC=y
> CONFIG_MTD_UBI=y
> CONFIG_BLK_DEV_LOOP=y
> CONFIG_ATMEL_SSC=m
> diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
> index fc1b69256b64..33572998dbbe 100644
> --- a/arch/arm/configs/multi_v7_defconfig
> +++ b/arch/arm/configs/multi_v7_defconfig
> @@ -217,7 +217,7 @@ CONFIG_MTD_NAND_DAVINCI=y
> CONFIG_MTD_NAND_STM32_FMC2=y
> CONFIG_MTD_NAND_PL35X=y
> CONFIG_MTD_SPI_NOR=y
> -CONFIG_SPI_ASPEED_SMC_MTD_SPI_NOR=m
> +CONFIG_SPI_ASPEED_SMC=m
> CONFIG_MTD_UBI=y
> CONFIG_BLK_DEV_LOOP=y
> CONFIG_BLK_DEV_RAM=y
> --
> 2.34.1
>
On Mon, 14 Feb 2022 at 09:42, Cédric Le Goater <[email protected]> wrote:
>
> Hi,
>
> This series adds a new SPI driver using the spi-mem interface for the
> Aspeed static memory controllers of the AST2600, AST2500 and AST2400
> SoCs.
>
> * AST2600 Firmware SPI Memory Controller (FMC)
> * AST2600 SPI Flash Controller (SPI1 and SPI2)
> * AST2500 Firmware SPI Memory Controller (FMC)
> * AST2500 SPI Flash Controller (SPI1 and SPI2)
> * AST2400 New Static Memory Controller (also referred as FMC)
> * AST2400 SPI Flash Controller (SPI)
>
> It is based on the current OpenBMC kernel driver [1], using directly
> the MTD SPI-NOR interface and on a patchset [2] previously proposed
> adding support for the AST2600 only. This driver takes a slightly
> different approach to cover all 6 controllers.
>
> It does not make use of the controller register disabling Address and
> Data byte lanes because is not available on the AST2400 SoC. We could
> introduce a specific handler for new features available on recent SoCs
> if needed. As there is not much difference on performance, the driver
> chooses the common denominator: "User mode" which has been heavily
> tested in [1]. "User mode" is also used as a fall back method when
> flash device mapping window is too small.
>
> Problems to address with spi-mem were the configuration of the mapping
> windows and the calibration of the read timings. The driver handles
> them in the direct mapping handler when some knowledge on the size of
> the flash device is know. It is not perfect but not incorrect either.
> The algorithm is one from [1] because it doesn't require the DMA
> registers which are not available on all controllers.
>
> Direct mapping for writes is not supported (yet). I have seen some
> corruption with writes and I preferred to use the safer and proven
> method of the initial driver [1]. We can improve that later.
>
> The driver supports Quad SPI RX transfers on the AST2600 SoC but it
> didn't have the expected results. Therefore it is not activated yet.
> This needs more tests.
>
> The series does not remove the current Aspeed SMC driver but prepares
> ground for its removal by changing its CONFIG option. This last step
> can be addressed as a followup when the new driver using the spi-mem
> interface has been sufficiently exposed.
>
> Tested on:
>
> * OpenPOWER Palmetto (AST2400)
> * Evaluation board (AST2500)
> * OpenPOWER Witherspoon (AST2500)
> * Evaluation board (AST2600 A0)
> * Rainier board (AST2600)
Looks great! Thanks for doing this work Cédric.
I reviewed all of the patches. The device tree and defconfig ones,
which we will send via my aspeed tree, are good to go.
The others look good too, to the best of my knowledge.
I'll do some more testing of your v2 when you send it out.
Cheers,
Joel
>
> [1] https://github.com/openbmc/linux/blob/dev-5.15/drivers/mtd/spi-nor/controllers/aspeed-smc.c
> [2] https://patchwork.ozlabs.org/project/linux-aspeed/list/?series=212394
>
> Thanks,
>
> C.
>
> Cédric Le Goater (10):
> mtd: spi-nor: aspeed: Rename Kconfig option
> dt-bindings: spi: Add Aspeed SMC controllers device tree binding
> spi: spi-mem: Add driver for Aspeed SMC controllers
> spi: aspeed: Add support for direct mapping
> spi: aspeed: Adjust direct mapping to device size
> spi: aspeed: Workaround AST2500 limitations
> spi: aspeed: Add support for the AST2400 SPI controller
> spi: aspeed: Calibrate read timings
> ARM: dts: aspeed: Enable Dual SPI RX transfers
> spi: aspeed: Activate new spi-mem driver
>
> drivers/spi/spi-aspeed-smc.c | 1241 +++++++++++++++++
> .../bindings/spi/aspeed,ast2600-fmc.yaml | 92 ++
> arch/arm/boot/dts/aspeed-g4.dtsi | 6 +
> arch/arm/boot/dts/aspeed-g5.dtsi | 7 +
> arch/arm/boot/dts/aspeed-g6.dtsi | 8 +
> drivers/mtd/spi-nor/controllers/Kconfig | 4 +-
> drivers/mtd/spi-nor/controllers/Makefile | 2 +-
> drivers/spi/Kconfig | 11 +
> drivers/spi/Makefile | 1 +
> 9 files changed, 1369 insertions(+), 3 deletions(-)
> create mode 100644 drivers/spi/spi-aspeed-smc.c
> create mode 100644 Documentation/devicetree/bindings/spi/aspeed,ast2600-fmc.yaml
>
> --
> 2.34.1
>
On 2/16/22 08:02, Joel Stanley wrote:
> On Mon, 14 Feb 2022 at 09:43, Cédric Le Goater <[email protected]> wrote:
>>
>> The previous driver using the MTD SPI NOR interface is kept in case we
>> find some issues but we should remove it quickly once the new driver
>> using the spi-mem interface has been sufficiently exposed.
>>
>> Signed-off-by: Cédric Le Goater <[email protected]>
>
> I suggest we drop the defconfig changes from both this patch and the
> first. This way we'll always have the new driver being built, with
> less churn.
>
> If you strongly prefer the way you've done it then that's fine too.
I am fine with that, but, with only patch 1, the defconfig files would
be referencing an non-existing CONFIG. Is that ok ?
Thanks,
C.
>
>> ---
>> arch/arm/configs/aspeed_g4_defconfig | 2 +-
>> arch/arm/configs/aspeed_g5_defconfig | 2 +-
>> arch/arm/configs/multi_v5_defconfig | 2 +-
>> arch/arm/configs/multi_v7_defconfig | 2 +-
>> 4 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm/configs/aspeed_g4_defconfig b/arch/arm/configs/aspeed_g4_defconfig
>> index 964536444cd7..b4a1b2ed1a17 100644
>> --- a/arch/arm/configs/aspeed_g4_defconfig
>> +++ b/arch/arm/configs/aspeed_g4_defconfig
>> @@ -64,7 +64,7 @@ CONFIG_MTD_BLOCK=y
>> CONFIG_MTD_PARTITIONED_MASTER=y
>> CONFIG_MTD_SPI_NOR=y
>> # CONFIG_MTD_SPI_NOR_USE_4K_SECTORS is not set
>> -CONFIG_SPI_ASPEED_SMC_MTD_SPI_NOR=y
>> +CONFIG_SPI_ASPEED_SMC=y
>> CONFIG_MTD_UBI=y
>> CONFIG_MTD_UBI_FASTMAP=y
>> CONFIG_MTD_UBI_BLOCK=y
>> diff --git a/arch/arm/configs/aspeed_g5_defconfig b/arch/arm/configs/aspeed_g5_defconfig
>> index e809236ca88b..ccc4240ee4b5 100644
>> --- a/arch/arm/configs/aspeed_g5_defconfig
>> +++ b/arch/arm/configs/aspeed_g5_defconfig
>> @@ -103,7 +103,7 @@ CONFIG_MTD_BLOCK=y
>> CONFIG_MTD_PARTITIONED_MASTER=y
>> CONFIG_MTD_SPI_NOR=y
>> # CONFIG_MTD_SPI_NOR_USE_4K_SECTORS is not set
>> -CONFIG_SPI_ASPEED_SMC_MTD_SPI_NOR=y
>> +CONFIG_SPI_ASPEED_SMC=y
>> CONFIG_MTD_UBI=y
>> CONFIG_MTD_UBI_FASTMAP=y
>> CONFIG_MTD_UBI_BLOCK=y
>> diff --git a/arch/arm/configs/multi_v5_defconfig b/arch/arm/configs/multi_v5_defconfig
>> index 49083ef05fb0..80a3ae02d759 100644
>> --- a/arch/arm/configs/multi_v5_defconfig
>> +++ b/arch/arm/configs/multi_v5_defconfig
>> @@ -103,7 +103,7 @@ CONFIG_MTD_RAW_NAND=y
>> CONFIG_MTD_NAND_ATMEL=y
>> CONFIG_MTD_NAND_ORION=y
>> CONFIG_MTD_SPI_NOR=y
>> -CONFIG_SPI_ASPEED_SMC_MTD_SPI_NOR=y
>> +CONFIG_SPI_ASPEED_SMC=y
>> CONFIG_MTD_UBI=y
>> CONFIG_BLK_DEV_LOOP=y
>> CONFIG_ATMEL_SSC=m
>> diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
>> index fc1b69256b64..33572998dbbe 100644
>> --- a/arch/arm/configs/multi_v7_defconfig
>> +++ b/arch/arm/configs/multi_v7_defconfig
>> @@ -217,7 +217,7 @@ CONFIG_MTD_NAND_DAVINCI=y
>> CONFIG_MTD_NAND_STM32_FMC2=y
>> CONFIG_MTD_NAND_PL35X=y
>> CONFIG_MTD_SPI_NOR=y
>> -CONFIG_SPI_ASPEED_SMC_MTD_SPI_NOR=m
>> +CONFIG_SPI_ASPEED_SMC=m
>> CONFIG_MTD_UBI=y
>> CONFIG_BLK_DEV_LOOP=y
>> CONFIG_BLK_DEV_RAM=y
>> --
>> 2.34.1
>>
On 2/15/22 22:06, Rob Herring wrote:
> On Mon, Feb 14, 2022 at 10:42:23AM +0100, Cédric Le Goater wrote:
>> The "interrupt" property is optional because it is only necessary for
>> controllers supporting DMAs (Not implemented yet in the new driver).
>>
>> Cc: Chin-Ting Kuo <[email protected]>
>> Signed-off-by: Cédric Le Goater <[email protected]>
>> ---
>> .../bindings/spi/aspeed,ast2600-fmc.yaml | 92 +++++++++++++++++++
>> 1 file changed, 92 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/spi/aspeed,ast2600-fmc.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/spi/aspeed,ast2600-fmc.yaml b/Documentation/devicetree/bindings/spi/aspeed,ast2600-fmc.yaml
>> new file mode 100644
>> index 000000000000..ed71c4d86930
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/spi/aspeed,ast2600-fmc.yaml
>> @@ -0,0 +1,92 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/spi/aspeed,ast2600-fmc.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Aspeed SMC controllers bindings
>> +
>> +maintainers:
>> + - Chin-Ting Kuo <[email protected]>
>> + - Cédric Le Goater <[email protected]>
>> +
>> +description: |
>> + This binding describes the Aspeed Static Memory Controllers (FMC and
>> + SPI) of the AST2400, AST2500 and AST2600 SOCs.
>> +
>> +allOf:
>> + - $ref: "spi-controller.yaml#"
>> +
>> +properties:
>> + compatible:
>> + enum:
>> + - aspeed,ast2600-fmc
>> + - aspeed,ast2600-spi
>> + - aspeed,ast2500-fmc
>> + - aspeed,ast2500-spi
>> + - aspeed,ast2400-fmc
>> + - aspeed,ast2400-spi
>> +
>> + reg:
>> + items:
>> + - description: registers
>> + - description: memory mapping
>> +
>> + clocks:
>> + maxItems: 1
>> +
>> + interrupts:
>> + maxItems: 1
>> +
>> +patternProperties:
>> + "@[0-9a-f]+":
>> + type: object
>> +
>> + properties:
>> + spi-rx-bus-width:
>> + enum: [1, 2, 4]
>> +
>> + required:
>> + - reg
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - clocks
>> +
>> +unevaluatedProperties: false
>> +
>> +examples:
>> + - |
>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>> + #include <dt-bindings/interrupt-controller/aspeed-scu-ic.h>
>> + #include <dt-bindings/clock/ast2600-clock.h>
>> +
>> + spi@1e620000 {
>> + reg = < 0x1e620000 0xc4
>> + 0x20000000 0x10000000 >;
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + compatible = "aspeed,ast2600-fmc";
>> + clocks = <&syscon ASPEED_CLK_AHB>;
>> + status = "disabled";
>
> Why is your example disabled? Drop 'status'.
my bad. I took the basic definition of the SoC and the devices
are activated in the boards. I will fix in v2.
Thanks,
C.
>
>> + interrupts = <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>;
>> + flash@0 {
>> + reg = < 0 >;
>> + compatible = "jedec,spi-nor";
>> + spi-max-frequency = <50000000>;
>> + status = "disabled";
>
> Ditto.
>
>> + };
>> + flash@1 {
>> + reg = < 1 >;
>> + compatible = "jedec,spi-nor";
>> + spi-max-frequency = <50000000>;
>> + status = "disabled";
>> + };
>> + flash@2 {
>> + reg = < 2 >;
>> + compatible = "jedec,spi-nor";
>> + spi-max-frequency = <50000000>;
>> + status = "disabled";
>> + };
>> + };
>> --
>> 2.34.1
>>
>>
On 14/02/22 10:42AM, C?dric Le Goater wrote:
> 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 a bit 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 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.
>
> Due to the lack of API, calibration is performed when the direct mapping
> for reads is created.
The dirmap_create mapping says nothing about _when_ it should be called.
So there is no guarantee that it will only be called after the flash is
fully initialized. I suggest you either make this a requirement of the
API, or create a new API that guarantees it will only be called after
the flash is initialized, like [0].
[0] https://patchwork.ozlabs.org/project/linux-mtd/patch/[email protected]/
>
> Cc: Pratyush Yadav <[email protected]>
> Signed-off-by: C?dric Le Goater <[email protected]>
> ---
--
Regards,
Pratyush Yadav
Texas Instruments Inc.
On 2/25/22 10:18, Pratyush Yadav wrote:
> On 14/02/22 10:42AM, Cédric Le Goater wrote:
>> 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 a bit 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 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.
>>
>> Due to the lack of API, calibration is performed when the direct mapping
>> for reads is created.
>
> The dirmap_create mapping says nothing about _when_ it should be called.
> So there is no guarantee that it will only be called after the flash is
> fully initialized.
spi_nor_create_read_dirmap() is called after spi_nor_scan() in spi_nor_probe().
Since a spi_mem_dirmap_info descriptor is created using the nor fields :
struct spi_mem_dirmap_info info = {
.op_tmpl = SPI_MEM_OP(SPI_MEM_OP_CMD(nor->read_opcode, 0),
SPI_MEM_OP_ADDR(nor->addr_width, 0, 0),
SPI_MEM_OP_DUMMY(nor->read_dummy, 0),
SPI_MEM_OP_DATA_IN(0, NULL, 0)),
.offset = 0,
.length = nor->params->size,
};
struct spi_mem_op *op = &info.op_tmpl;
the spi-mem framework makes the assumption that the nor object is initialized.
> I suggest you either make this a requirement of the API,
how ?
Thanks,
C.
> or create a new API that guarantees it will only be called after
> the flash is initialized, like [0].
>
> [0] https://patchwork.ozlabs.org/project/linux-mtd/patch/[email protected]/
>
>>
>> Cc: Pratyush Yadav <[email protected]>
>> Signed-off-by: Cédric Le Goater <[email protected]>
>> ---
>