All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mtd/spi-nor: Add SPI memory controllers for Aspeed SoCs
@ 2016-10-17 16:57 ` Cédric Le Goater
  0 siblings, 0 replies; 12+ messages in thread
From: Cédric Le Goater @ 2016-10-17 16:57 UTC (permalink / raw)
  To: linux-mtd
  Cc: devicetree, Milton Miller, Rob Herring, Joel Stanley,
	Brian Norris, David Woodhouse, Cédric Le Goater

This driver adds mtd support for spi-nor attached to either or both of
the Firmware Memory Controller or the SPI Flash Controller (AST2400
only).

The SMC controllers on the Aspeed AST2500 SoC are very similar to the
ones found on the AST2400. The differences are on the number of
supported flash modules and their default mappings in the SoC address
space.

The Aspeed AST2500 has one SPI controller for the BMC firmware and two
for the host firmware. All controllers have now the same set of
registers compatible with the AST2400 FMC controller and the legacy
'SMC' controller is fully gone.

Based on previous work from Milton D. Miller II <miltonm@us.ibm.com>

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 .../devicetree/bindings/mtd/aspeed-smc.txt         |  86 +++
 drivers/mtd/spi-nor/Kconfig                        |  12 +
 drivers/mtd/spi-nor/Makefile                       |   1 +
 drivers/mtd/spi-nor/aspeed-smc.c                   | 746 +++++++++++++++++++++
 4 files changed, 845 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/aspeed-smc.txt
 create mode 100644 drivers/mtd/spi-nor/aspeed-smc.c

diff --git a/Documentation/devicetree/bindings/mtd/aspeed-smc.txt b/Documentation/devicetree/bindings/mtd/aspeed-smc.txt
new file mode 100644
index 000000000000..f6bfa7761205
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/aspeed-smc.txt
@@ -0,0 +1,86 @@
+* Aspeed Static Memory controller
+* Aspeed SPI Flash Controller
+
+The Static memory controller in the ast2400 supports 5 chip selects each
+can be attached to NAND, parallel NOR, or SPI NOR attached flash.  The
+Firmware Memory Controller in the ast2500 supports 3 chip selects, two of
+which are always in SPI-NOR mode and the third can be SPI-NOR or parallel
+flash.  The SPI flash controller in the ast2400 supports one of 2 chip
+selects selected by pinmux.  The two SPI flash controllers in the ast2500
+each support two chip selects.
+
+Required properties:
+  - compatible : Should be one of
+	"aspeed,ast2400-fmc" for the AST2400 Static Memory Controller
+	"aspeed,ast2400-smc" for the AST2400 SPI Flash Controller
+	"aspeed,ast2500-fmc" for the AST2500 Firmware SPI Memory Controller
+	"aspeed,ast2500-smc" for the AST2500 SPI Flash Controllers
+  - reg : the first contains the control register location and length,
+          the second contains the memory window mapping address and length
+  - clocks : The APB clock input to the controller
+  - #address-cells : must be 1 corresponding to chip select child binding
+  - #size-cells : must be 0 corresponding to chip select child binding
+
+Optional properties:
+  - aspeed,fmc-has-dma : controller supports DMA transfers
+  - interrupts : Should contain the interrupt for the dma device if an fmc
+
+Child node required properties:
+  - reg : must contain chip select number in first cell of address, must
+	  be 1 tuple long
+  - compatible : may contain "vendor,part", must include "jedec,spi-nor"
+		when attached to SPI flash (see spi-nor.txt binding).
+
+Child node optional properties:
+  - label           - (optional) name to assign to mtd, default os assigned
+
+Child node optional properties for SPI mode (may be ignored):
+  - spi-max-frequency - (optional) max frequency of spi bus
+  - spi-cpol        - (optional) Empty property indicating device requires
+			inverse clock polarity (CPOL) mode (boolean)
+  - spi-cpha        - (optional) Empty property indicating device requires
+			shifted clock phase (CPHA) mode (boolean)
+  - spi-tx-bus-width - (optional) The bus width(number of data wires) that
+                        used for MOSI. Defaults to 1 if not present.
+  - spi-rx-bus-width - (optional) The bus width(number of data wires) that
+                        used for MOSI. Defaults to 1 if not present.
+
+Grandchild node optional properties:
+ - see mtd/partiton.txt for partitioning bindings and mtd naming
+
+
+Example:
+
+fmc: fmc@1e620000 {
+	compatible = "aspeed,ast2400-fmc";
+	reg = < 0x1e620000 0x94
+		0x20000000 0x02000000
+		0x22000000 0x02000000 >;
+	#address-cells = <1>;
+	#size-cells = <0>;
+	flash@0 {
+		reg = < 0 >;
+		compatible = "jedec,spi-nor" ;
+		label = "bmc";
+		/* spi-max-frequency = <>; */
+		/* m25p,fast-read; */
+		#address-cells = <1>;
+		#size-cells = <1>;
+		boot@0 {
+			label = "boot-loader";
+			reg = < 0 0x8000 >
+		}
+		image@8000 {
+			label = "kernel-image";
+			reg = < 0x8000 0x1f8000 >
+		}
+	};
+	flash@1 {
+		reg = < 1 >;
+		compatible = "jedec,spi-nor" ;
+		label = "alt";
+		/* spi-max-frequency = <>; */
+		status = "fail";
+		/* m25p,fast-read; */
+	};
+};
diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
index 4a682ee0f632..96148600fdab 100644
--- a/drivers/mtd/spi-nor/Kconfig
+++ b/drivers/mtd/spi-nor/Kconfig
@@ -76,4 +76,16 @@ config SPI_NXP_SPIFI
 	  Flash. Enable this option if you have a device with a SPIFI
 	  controller and want to access the Flash as a mtd device.
 
+config ASPEED_FLASH_SPI
+	tristate "Aspeed flash controllers in SPI mode"
+	depends on HAS_IOMEM && OF
+	depends on ARCH_ASPEED || COMPILE_TEST
+	# IO_SPACE_LIMIT must be equivalent to (~0UL)
+	depends on !NEED_MACH_IO_H
+	help
+	  This enables support for the New Static Memory Controller
+	  (FMC) in the Aspeed SoCs (AST2400 and AST2500) when attached
+	  to SPI nor chips, and support for the SPI Memory controller
+	  (SPI) for the BIOS.
+
 endif # MTD_SPI_NOR
diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
index 121695e83542..c3174ebc45c2 100644
--- a/drivers/mtd/spi-nor/Makefile
+++ b/drivers/mtd/spi-nor/Makefile
@@ -4,4 +4,5 @@ obj-$(CONFIG_SPI_CADENCE_QUADSPI)	+= cadence-quadspi.o
 obj-$(CONFIG_SPI_FSL_QUADSPI)	+= fsl-quadspi.o
 obj-$(CONFIG_SPI_HISI_SFC)	+= hisi-sfc.o
 obj-$(CONFIG_MTD_MT81xx_NOR)    += mtk-quadspi.o
+obj-$(CONFIG_ASPEED_FLASH_SPI)	+= aspeed-smc.o
 obj-$(CONFIG_SPI_NXP_SPIFI)	+= nxp-spifi.o
diff --git a/drivers/mtd/spi-nor/aspeed-smc.c b/drivers/mtd/spi-nor/aspeed-smc.c
new file mode 100644
index 000000000000..a1b9d6711f73
--- /dev/null
+++ b/drivers/mtd/spi-nor/aspeed-smc.c
@@ -0,0 +1,746 @@
+/*
+ * ASPEED Static Memory Controller driver
+ *
+ * Copyright (c) 2015-2016, IBM Corporation.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include <linux/bug.h>
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/partitions.h>
+#include <linux/mtd/spi-nor.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/sysfs.h>
+
+#define DEVICE_NAME	"aspeed-smc"
+
+/*
+ * In user mode all data bytes read or written to the chip decode address
+ * range are transferred to or from the SPI bus. The range is treated as a
+ * fifo of arbitratry 1, 2, or 4 byte width but each write has to be aligned
+ * to its size.  The address within the multiple 8kB range is ignored when
+ * sending bytes to the SPI bus.
+ *
+ * On the arm architecture, as of Linux version 4.3, memcpy_fromio and
+ * memcpy_toio on little endian targets use the optimized memcpy routines
+ * that were designed for well behavied memory storage.  These routines
+ * have a stutter if the source and destination are not both word aligned,
+ * once with a duplicate access to the source after aligning to the
+ * destination to a word boundary, and again with a duplicate access to
+ * the source when the final byte count is not word aligned.
+ *
+ * When writing or reading the fifo this stutter discards data or sends
+ * too much data to the fifo and can not be used by this driver.
+ *
+ * While the low level io string routines that implement the insl family do
+ * the desired accesses and memory increments, the cross architecture io
+ * macros make them essentially impossible to use on a memory mapped address
+ * instead of a a token from the call to iomap of an io port.
+ *
+ * These fifo routines use readl and friends to a constant io port and update
+ * the memory buffer pointer and count via explicit code. The final updates
+ * to len are optimistically suppressed.
+ */
+
+static void aspeed_smc_from_fifo(void *buf, const void __iomem *iop, size_t len)
+{
+	if (!len)
+		return;
+
+	/* Expect a 4 byte input port.  Otherwise just read bytes. */
+	if (unlikely((unsigned long)iop & 3)) {
+		while (len--) {
+			*(u8 *)buf = readb(iop);
+			buf++;
+		}
+	}
+
+	/* Align target to word: first byte then half word */
+	if ((unsigned long)buf & 1) {
+		*(u8 *)buf = readb(iop);
+		buf++;
+		len--;
+	}
+	if (((unsigned long)buf & 2) && (len >= 2)) {
+		*(u16 *)buf = readw(iop);
+		buf += 2;
+		len -= 2;
+	}
+
+	/* Transfer words, then remaining halfword and remaining byte */
+	while (len >= 4) {
+		*(u32 *)buf = readl(iop);
+		buf += 4;
+		len -= 4;
+	}
+	if (len & 2) {
+		*(u16 *)buf = readw(iop);
+		buf += 2;
+	}
+	if (len & 1)
+		*(u8 *)buf = readb(iop);
+}
+
+static void aspeed_smc_to_fifo(void __iomem *iop, const void *buf, size_t len)
+{
+	if (!len)
+		return;
+
+	/* Expect a 4 byte output port.  Otherwise just write bytes. */
+	if ((unsigned long)iop & 3) {
+		while (len--) {
+			writeb(*(u8 *)buf, iop);
+			buf++;
+		}
+		return;
+	}
+
+	/* Align target to word: first byte then half word */
+	if ((unsigned long)buf & 1) {
+		writeb(*(u8 *)buf, iop);
+		buf++;
+		len--;
+	}
+	if (((unsigned long)buf & 2) && (len >= 2)) {
+		writew(*(u16 *)buf, iop);
+		buf += 2;
+		len -= 2;
+	}
+
+	/* Transfer words, then remaining halfword and remaining byte */
+	while (len >= 4) {
+		writel(*(u32 *)buf, iop);
+		buf += 4;
+		len -= 4;
+	}
+	if (len & 2) {
+		writew(*(u16 *)buf, iop);
+		buf += 2;
+	}
+	if (len & 1)
+		writeb(*(u8 *)buf, iop);
+}
+
+enum smc_flash_type {
+	smc_type_nor = 0,	/* controller connected to nor flash */
+	smc_type_nand = 1,	/* controller connected to nand flash */
+	smc_type_spi = 2,	/* controller connected to spi flash */
+};
+
+struct aspeed_smc_info {
+	u32 maxsize;		/* maximum size of 1 chip window */
+	u8 nce;			/* number of chip enables */
+	u8 maxwidth;		/* max width of spi bus */
+	bool hastype;		/* flash type field exists in cfg reg */
+	u8 we0;			/* shift for write enable bit for ce 0 */
+	u8 ctl0;		/* offset in regs of ctl for ce 0 */
+	u8 time;		/* offset in regs of timing */
+	u8 misc;		/* offset in regs of misc settings */
+};
+
+static struct aspeed_smc_info fmc_2400_info = {
+	.maxsize = 64 * 1024 * 1024,
+	.nce = 5,
+	.maxwidth = 4,
+	.hastype = true,
+	.we0 = 16,
+	.ctl0 = 0x10,
+	.time = 0x94,
+	.misc = 0x54,
+};
+
+static struct aspeed_smc_info smc_2400_info = {
+	.maxsize = 64 * 1024 * 1024,
+	.nce = 1,
+	.maxwidth = 2,
+	.hastype = false,
+	.we0 = 0,
+	.ctl0 = 0x04,
+	.time = 0x14,
+	.misc = 0x10,
+};
+
+static struct aspeed_smc_info fmc_2500_info = {
+	.maxsize = 256 * 1024 * 1024,
+	.nce = 3,
+	.maxwidth = 2,
+	.hastype = true,
+	.we0 = 16,
+	.ctl0 = 0x10,
+	.time = 0x94,
+	.misc = 0x54,
+};
+
+static struct aspeed_smc_info smc_2500_info = {
+	.maxsize = 128 * 1024 * 1024,
+	.nce = 2,
+	.maxwidth = 2,
+	.hastype = false,
+	.we0 = 16,
+	.ctl0 = 0x10,
+	.time = 0x94,
+	.misc = 0x54,
+};
+
+enum smc_ctl_reg_value {
+	smc_base,		/* base value without mode for other commands */
+	smc_read,		/* command reg for (maybe fast) reads */
+	smc_write,		/* command reg for writes with timings */
+	smc_num_ctl_reg_values	/* last value to get count of commands */
+};
+
+struct aspeed_smc_controller;
+
+struct aspeed_smc_chip {
+	int cs;
+	struct aspeed_smc_controller *controller;
+	__le32 __iomem *ctl;			/* control register */
+	void __iomem *base;			/* base of chip window */
+	__le32 ctl_val[smc_num_ctl_reg_values];	/* controls with timing */
+	enum smc_flash_type type;		/* what type of flash */
+	struct spi_nor nor;
+};
+
+struct aspeed_smc_controller {
+	struct device *dev;
+
+	struct mutex mutex;			/* controller access mutex */
+	const struct aspeed_smc_info *info;	/* type info of controller */
+	void __iomem *regs;			/* controller registers */
+	void __iomem *windows;			/* per-chip windows resource */
+
+	struct aspeed_smc_chip *chips[0];	/* pointers to attached chips */
+};
+
+/*
+ * FMC Type setting Register
+ *   or
+ * SPI Flash Configuration Register
+ */
+#define CONFIG_REG			0x0
+
+/*
+ * CE Control Register
+ */
+#define CE_CONTROL_REG			0x4
+#define    CE0_CONTROL_EXTENDED		    BIT(0)
+
+/* CE0 Control Register (depends on the controller type) */
+#define CONTROL_SPI_AAF_MODE BIT(31)
+#define CONTROL_SPI_IO_MODE_MASK GENMASK(30, 28)
+#define CONTROL_SPI_IO_DUAL_DATA BIT(29)
+#define CONTROL_SPI_IO_DUAL_ADDR_DATA (BIT(29) | BIT(28))
+#define CONTROL_SPI_IO_QUAD_DATA BIT(30)
+#define CONTROL_SPI_IO_QUAD_ADDR_DATA (BIT(30) | BIT(28))
+#define CONTROL_SPI_CE_INACTIVE_SHIFT 24
+#define CONTROL_SPI_CE_INACTIVE_MASK GENMASK(27, CONTROL_SPI_CE_INACTIVE_SHIFT)
+/* 0 = 16T ... 15 = 1T   T=HCLK */
+#define CONTROL_SPI_COMMAND_SHIFT 16
+#define CONTROL_SPI_DUMMY_CYCLE_COMMAND_OUTPUT BIT(15)
+#define CONTROL_SPI_IO_DUMMY_CYCLES_HI BIT(14)
+#define CONTROL_SPI_IO_DUMMY_CYCLES_HI_SHIFT (14 - 2)
+#define CONTROL_SPI_IO_ADDRESS_4B BIT(13) /* 2400-smc */
+#define CONTROL_SPI_CLK_DIV4 BIT(13) /* FMC, 2500 */
+#define CONTROL_SPI_RW_MERGE BIT(12)
+#define CONTROL_SPI_IO_DUMMY_CYCLES_LO_SHIFT 6
+#define CONTROL_SPI_IO_DUMMY_CYCLES_LO GENMASK(7, \
+				       CONTROL_SPI_IO_DUMMY_CYCLES_LO_SHIFT)
+#define CONTROL_SPI_IO_DUMMY_CYCLES_MASK (CONTROL_SPI_IO_DUMMY_CYCLES_HI | \
+					  CONTROL_SPI_IO_DUMMY_CYCLES_LO)
+#define CONTROL_SPI_CLOCK_FREQ_SEL_SHIFT 8
+#define CONTROL_SPI_CLOCK_FREQ_SEL_MASK GENMASK(11, \
+					CONTROL_SPI_CLOCK_FREQ_SEL_SHIFT)
+#define CONTROL_SPI_LSB_FIRST BIT(5)
+#define CONTROL_SPI_CLOCK_MODE_3 BIT(4)
+#define CONTROL_SPI_IN_DUAL_DATA BIT(3)
+#define CONTROL_SPI_CE_STOP_ACTIVE_CONTROL BIT(2)
+#define CONTROL_SPI_COMMAND_MODE_MASK GENMASK(1, 0)
+#define CONTROL_SPI_COMMAND_MODE_NORMAL (0)
+#define CONTROL_SPI_COMMAND_MODE_FREAD (1)
+#define CONTROL_SPI_COMMAND_MODE_WRITE (2)
+#define CONTROL_SPI_COMMAND_MODE_USER (3)
+
+#define CONTROL_SPI_KEEP_MASK (CONTROL_SPI_AAF_MODE | \
+	CONTROL_SPI_CE_INACTIVE_MASK | CONTROL_SPI_CLK_DIV4 | \
+	CONTROL_SPI_IO_DUMMY_CYCLES_MASK | CONTROL_SPI_CLOCK_FREQ_SEL_MASK | \
+	CONTROL_SPI_LSB_FIRST | CONTROL_SPI_CLOCK_MODE_3)
+
+/* Segment Address Registers */
+#define SEGMENT_ADDR_REG0		0x30
+#define     SEGMENT_ADDR_START(_r)	    ((((_r) >> 16) & 0xFF) << 23)
+#define     SEGMENT_ADDR_END(_r)	    ((((_r) >> 24) & 0xFF) << 23)
+
+static u32 spi_control_fill_opcode(u8 opcode)
+{
+	return ((u32)(opcode)) << CONTROL_SPI_COMMAND_SHIFT;
+}
+
+static inline u32 aspeed_smc_chip_write_bit(struct aspeed_smc_chip *chip)
+{
+	return ((u32)1 << (chip->controller->info->we0 + chip->cs));
+}
+
+static void aspeed_smc_chip_check_config(struct aspeed_smc_chip *chip)
+{
+	struct aspeed_smc_controller *controller = chip->controller;
+	u32 reg;
+
+	reg = readl(controller->regs + CONFIG_REG);
+
+	if (!(reg & aspeed_smc_chip_write_bit(chip))) {
+		dev_dbg(controller->dev,
+			"config write is not set ! @%p: 0x%08x\n",
+			controller->regs + CONFIG_REG, reg);
+		reg |= aspeed_smc_chip_write_bit(chip);
+		writel(reg, controller->regs + CONFIG_REG);
+	}
+}
+
+static void aspeed_smc_start_user(struct spi_nor *nor)
+{
+	struct aspeed_smc_chip *chip = nor->priv;
+	u32 ctl = chip->ctl_val[smc_base];
+
+	/*
+	 * When the chip is controlled in user mode, we need write
+	 * access to send the opcodes to it. So check the config.
+	 */
+	aspeed_smc_chip_check_config(chip);
+
+	ctl |= CONTROL_SPI_COMMAND_MODE_USER |
+		CONTROL_SPI_CE_STOP_ACTIVE_CONTROL;
+	writel(ctl, chip->ctl);
+
+	ctl &= ~CONTROL_SPI_CE_STOP_ACTIVE_CONTROL;
+	writel(ctl, chip->ctl);
+}
+
+static void aspeed_smc_stop_user(struct spi_nor *nor)
+{
+	struct aspeed_smc_chip *chip = nor->priv;
+
+	u32 ctl = chip->ctl_val[smc_read];
+	u32 ctl2 = ctl | CONTROL_SPI_COMMAND_MODE_USER |
+		CONTROL_SPI_CE_STOP_ACTIVE_CONTROL;
+
+	writel(ctl2, chip->ctl);	/* stop user CE control */
+	writel(ctl, chip->ctl);		/* default to fread or read */
+}
+
+static int aspeed_smc_read_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
+{
+	struct aspeed_smc_chip *chip = nor->priv;
+
+	mutex_lock(&chip->controller->mutex);
+
+	aspeed_smc_start_user(nor);
+	aspeed_smc_to_fifo(chip->base, &opcode, 1);
+	aspeed_smc_from_fifo(buf, chip->base, len);
+	aspeed_smc_stop_user(nor);
+
+	mutex_unlock(&chip->controller->mutex);
+
+	return 0;
+}
+
+static int aspeed_smc_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf,
+				int len)
+{
+	struct aspeed_smc_chip *chip = nor->priv;
+
+	mutex_lock(&chip->controller->mutex);
+
+	aspeed_smc_start_user(nor);
+	aspeed_smc_to_fifo(chip->base, &opcode, 1);
+	aspeed_smc_to_fifo(chip->base, buf, len);
+	aspeed_smc_stop_user(nor);
+
+	mutex_unlock(&chip->controller->mutex);
+
+	return 0;
+}
+
+static void aspeed_smc_send_cmd_addr(struct spi_nor *nor, u8 cmd, u32 addr)
+{
+	struct aspeed_smc_chip *chip = nor->priv;
+	__be32 temp;
+	u32 cmdaddr;
+
+	switch (nor->addr_width) {
+	default:
+		WARN_ONCE(1, "Unexpected address width %u, defaulting to 3\n",
+			  nor->addr_width);
+		/* FALLTHROUGH */
+	case 3:
+		cmdaddr = addr & 0xFFFFFF;
+
+		cmdaddr |= (u32)cmd << 24;
+
+		temp = cpu_to_be32(cmdaddr);
+		aspeed_smc_to_fifo(chip->base, &temp, 4);
+		break;
+	case 4:
+		temp = cpu_to_be32(addr);
+		aspeed_smc_to_fifo(chip->base, &cmd, 1);
+		aspeed_smc_to_fifo(chip->base, &temp, 4);
+		break;
+	}
+}
+
+static ssize_t aspeed_smc_read_user(struct spi_nor *nor, loff_t from, size_t len,
+				    u_char *read_buf)
+{
+	struct aspeed_smc_chip *chip = nor->priv;
+
+	mutex_lock(&chip->controller->mutex);
+
+	aspeed_smc_start_user(nor);
+	aspeed_smc_send_cmd_addr(nor, nor->read_opcode, from);
+	aspeed_smc_from_fifo(read_buf, chip->base, len);
+	aspeed_smc_stop_user(nor);
+
+	mutex_unlock(&chip->controller->mutex);
+
+	return len;
+}
+
+static ssize_t aspeed_smc_write_user(struct spi_nor *nor, loff_t to, size_t len,
+				     const u_char *write_buf)
+{
+	struct aspeed_smc_chip *chip = nor->priv;
+
+	mutex_lock(&chip->controller->mutex);
+
+	aspeed_smc_start_user(nor);
+	aspeed_smc_send_cmd_addr(nor, nor->program_opcode, to);
+	aspeed_smc_to_fifo(chip->base, write_buf, len);
+	aspeed_smc_stop_user(nor);
+
+	mutex_unlock(&chip->controller->mutex);
+
+	return len;
+}
+
+static int aspeed_smc_remove(struct platform_device *dev)
+{
+	struct aspeed_smc_chip *chip;
+	struct aspeed_smc_controller *controller = platform_get_drvdata(dev);
+	int n;
+
+	for (n = 0; n < controller->info->nce; n++) {
+		chip = controller->chips[n];
+		if (chip)
+			mtd_device_unregister(&chip->nor.mtd);
+	}
+
+	return 0;
+}
+
+const struct of_device_id aspeed_smc_matches[] = {
+	{ .compatible = "aspeed,ast2400-fmc", .data = &fmc_2400_info },
+	{ .compatible = "aspeed,ast2400-smc", .data = &smc_2400_info },
+	{ .compatible = "aspeed,ast2500-fmc", .data = &fmc_2500_info },
+	{ .compatible = "aspeed,ast2500-smc", .data = &smc_2500_info },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, aspeed_smc_matches);
+
+static struct platform_device *
+of_platform_device_create_or_find(struct device_node *child,
+				  struct device *parent)
+{
+	struct platform_device *cdev;
+
+	cdev = of_platform_device_create(child, NULL, parent);
+	if (!cdev)
+		cdev = of_find_device_by_node(child);
+	return cdev;
+}
+
+static void __iomem *window_start(struct aspeed_smc_controller *controller,
+				  struct resource *r, unsigned int n)
+{
+	u32 offset = 0;
+	u32 reg;
+
+	if (controller->info->nce > 1) {
+		reg = readl(controller->regs + SEGMENT_ADDR_REG0 + n * 4);
+
+		if (SEGMENT_ADDR_START(reg) >= SEGMENT_ADDR_END(reg))
+			return NULL;
+
+		offset = SEGMENT_ADDR_START(reg) - r->start;
+	}
+
+	return controller->windows + offset;
+}
+
+static void aspeed_smc_chip_enable_write(struct aspeed_smc_chip *chip)
+{
+	struct aspeed_smc_controller *controller = chip->controller;
+	u32 reg;
+
+	reg = readl(controller->regs + CONFIG_REG);
+	dev_dbg(controller->dev, "config reg @%p: 0x%08x\n",
+		controller->regs + CONFIG_REG, reg);
+
+	reg |= aspeed_smc_chip_write_bit(chip);
+	writel(reg, controller->regs + CONFIG_REG);
+}
+
+static void aspeed_smc_chip_set_type(struct aspeed_smc_chip *chip, int type)
+{
+	struct aspeed_smc_controller *controller = chip->controller;
+	u32 reg;
+
+	reg = readl(controller->regs + CONFIG_REG);
+	dev_dbg(controller->dev, "config reg @%p: 0x%08x\n",
+		controller->regs + CONFIG_REG, reg);
+
+	chip->type = type;
+
+	reg &= ~(3 << (chip->cs * 2));
+	reg |= chip->type << (chip->cs * 2);
+	writel(reg, controller->regs + CONFIG_REG);
+}
+
+static int aspeed_smc_chip_setup_init(struct aspeed_smc_chip *chip,
+				      struct resource *r)
+{
+	struct aspeed_smc_controller *controller = chip->controller;
+	const struct aspeed_smc_info *info = controller->info;
+	u32 reg;
+
+	/*
+	 * Always turn on the write enable bit to allow opcodes to be
+	 * sent in user mode.
+	 */
+	aspeed_smc_chip_enable_write(chip);
+
+	/* The driver only supports SPI type flash for the moment */
+	if (info->hastype)
+		aspeed_smc_chip_set_type(chip, smc_type_spi);
+
+	/*
+	 * Configure chip base address in memory
+	 */
+	chip->base = window_start(controller, r, chip->cs);
+	if (!chip->base) {
+		dev_warn(chip->nor.dev, "CE segment window closed.\n");
+		return -1;
+	}
+
+	/*
+	 * Read the existing control register to get basic values.
+	 *
+	 * XXX This register probably needs more sanitation.
+	 *
+	 * Do we need support for mode 3 vs mode 0 clock phasing?
+	 */
+	reg = readl(chip->ctl);
+	dev_dbg(controller->dev, "control register: %08x\n", reg);
+
+	if ((reg & CONTROL_SPI_KEEP_MASK) != reg) {
+		chip->ctl_val[smc_base] = reg & CONTROL_SPI_KEEP_MASK;
+		dev_info(controller->dev,
+			 "control register changed to: %08x\n",
+			 chip->ctl_val[smc_base]);
+	}
+
+	/*
+	 * 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_SPI_COMMAND_MODE_MASK) ==
+	    CONTROL_SPI_COMMAND_MODE_NORMAL)
+		chip->ctl_val[smc_read] = reg;
+	else
+		chip->ctl_val[smc_read] = chip->ctl_val[smc_base] |
+			CONTROL_SPI_COMMAND_MODE_NORMAL;
+
+	return 0;
+}
+
+static void aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip)
+{
+	struct aspeed_smc_controller *controller = chip->controller;
+	u32 reg;
+
+	/*
+	 * Set 4 byte mode in the chip controller register and also in
+	 * controller config register. The BMC flash controller is
+	 * strapped by hardware, or autodetected, but the SPI flash
+	 * controller of the AST2500 still needs to be set.
+	 */
+	if (chip->nor.mtd.size > SZ_16M) {
+		chip->ctl_val[smc_base] |= CONTROL_SPI_IO_ADDRESS_4B;
+
+		/*
+		 * The SPI flash controller of the AST2400 does not
+		 * have such a setting.
+		 */
+		if (chip->controller->info == &smc_2500_info) {
+			reg = readl(controller->regs + CE_CONTROL_REG);
+			reg |= 1 << chip->cs;
+			writel(reg, controller->regs + CE_CONTROL_REG);
+		}
+	}
+
+	chip->ctl_val[smc_write] = chip->ctl_val[smc_base] |
+		spi_control_fill_opcode(chip->nor.program_opcode) |
+		CONTROL_SPI_COMMAND_MODE_WRITE;
+
+	/*
+	 * XXX TODO
+	 * Enable fast read mode as required here.
+	 * Adjust clocks if fast read and write are supported.
+	 * Interpret spi-nor flags to adjust controller settings.
+	 * Check if resource size big enough for detected chip and
+	 * add support assisted (normal or fast-) read and dma.
+	 */
+}
+
+static int aspeed_smc_probe(struct platform_device *pdev)
+{
+	struct aspeed_smc_controller *controller;
+	const struct of_device_id *match;
+	const struct aspeed_smc_info *info;
+	struct resource *r;
+	struct device_node *child;
+	int err = 0;
+	unsigned int n;
+
+	match = of_match_device(aspeed_smc_matches, &pdev->dev);
+	if (!match || !match->data)
+		return -ENODEV;
+	info = match->data;
+
+	controller = devm_kzalloc(&pdev->dev, sizeof(*controller) +
+		info->nce * sizeof(controller->chips[0]), GFP_KERNEL);
+	if (!controller)
+		return -ENOMEM;
+	controller->info = info;
+
+	mutex_init(&controller->mutex);
+	platform_set_drvdata(pdev, controller);
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	controller->regs = devm_ioremap_resource(&pdev->dev, r);
+	if (IS_ERR(controller->regs))
+		return PTR_ERR(controller->regs);
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	controller->windows = devm_ioremap_resource(&pdev->dev, r);
+	if (IS_ERR(controller->windows))
+		return PTR_ERR(controller->windows);
+
+	controller->dev = &pdev->dev;
+
+	/* The pinmux or bootloader will disable the legacy mode controller */
+
+	/*
+	 * XXX Need to add arbitration to the SMC (BIOS) controller if access
+	 * is shared by the host.
+	 */
+	for_each_available_child_of_node(controller->dev->of_node, child) {
+		struct platform_device *cdev;
+		struct aspeed_smc_chip *chip;
+
+		/* This version does not support nand or nor flash devices. */
+		if (!of_device_is_compatible(child, "jedec,spi-nor"))
+			continue;
+
+		/*
+		 * create a platform device from the of node.  If the device
+		 * already was created (eg from a prior bind/unbind cycle)
+		 * reuse it.
+		 *
+		 * The creating the device node for the child here allows its
+		 * use for error reporting via dev_err below.
+		 */
+		cdev = of_platform_device_create_or_find(child,
+							 controller->dev);
+		if (!cdev)
+			continue;
+
+		err = of_property_read_u32(child, "reg", &n);
+		if (err == -EINVAL && info->nce == 1)
+			n = 0;
+		else if (err || n >= info->nce)
+			continue;
+		if (controller->chips[n]) {
+			dev_err(&cdev->dev,
+				"chip-id %u already in use in use by %s\n",
+				n, dev_name(controller->chips[n]->nor.dev));
+			continue;
+		}
+
+		chip = devm_kzalloc(controller->dev, sizeof(*chip), GFP_KERNEL);
+		if (!chip)
+			continue;
+		chip->controller = controller;
+		chip->ctl = controller->regs + info->ctl0 + n * 4;
+		chip->cs = n;
+
+		chip->nor.dev = &cdev->dev;
+		chip->nor.priv = chip;
+		spi_nor_set_flash_node(&chip->nor, child);
+		chip->nor.mtd.name = of_get_property(child, "label", NULL);
+		chip->nor.read = aspeed_smc_read_user;
+		chip->nor.write = aspeed_smc_write_user;
+		chip->nor.read_reg = aspeed_smc_read_reg;
+		chip->nor.write_reg = aspeed_smc_write_reg;
+
+		aspeed_smc_chip_setup_init(chip, r);
+
+		/*
+		 * XXX Add support for SPI_NOR_QUAD and SPI_NOR_DUAL attach
+		 * when board support is present as determined by of property.
+		 */
+		err = spi_nor_scan(&chip->nor, NULL, SPI_NOR_NORMAL);
+		if (err)
+			continue;
+
+		aspeed_smc_chip_setup_finish(chip);
+
+		err = mtd_device_register(&chip->nor.mtd, NULL, 0);
+		if (err)
+			continue;
+		controller->chips[n] = chip;
+	}
+
+	/* Were any children registered? */
+	for (n = 0; n < info->nce; n++)
+		if (controller->chips[n])
+			break;
+
+	if (n == info->nce)
+		return -ENODEV;
+
+	return 0;
+}
+
+static struct platform_driver aspeed_smc_driver = {
+	.probe = aspeed_smc_probe,
+	.remove = aspeed_smc_remove,
+	.driver = {
+		.name = DEVICE_NAME,
+		.of_match_table = aspeed_smc_matches,
+	}
+};
+
+module_platform_driver(aspeed_smc_driver);
+
+MODULE_DESCRIPTION("ASPEED Static Memory Controller Driver");
+MODULE_AUTHOR("Milton Miller");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH] mtd/spi-nor: Add SPI memory controllers for Aspeed SoCs
@ 2016-10-17 16:57 ` Cédric Le Goater
  0 siblings, 0 replies; 12+ messages in thread
From: Cédric Le Goater @ 2016-10-17 16:57 UTC (permalink / raw)
  To: linux-mtd
  Cc: David Woodhouse, Brian Norris, Rob Herring, devicetree,
	Joel Stanley, Milton Miller, Cédric Le Goater

This driver adds mtd support for spi-nor attached to either or both of
the Firmware Memory Controller or the SPI Flash Controller (AST2400
only).

The SMC controllers on the Aspeed AST2500 SoC are very similar to the
ones found on the AST2400. The differences are on the number of
supported flash modules and their default mappings in the SoC address
space.

The Aspeed AST2500 has one SPI controller for the BMC firmware and two
for the host firmware. All controllers have now the same set of
registers compatible with the AST2400 FMC controller and the legacy
'SMC' controller is fully gone.

Based on previous work from Milton D. Miller II <miltonm@us.ibm.com>

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 .../devicetree/bindings/mtd/aspeed-smc.txt         |  86 +++
 drivers/mtd/spi-nor/Kconfig                        |  12 +
 drivers/mtd/spi-nor/Makefile                       |   1 +
 drivers/mtd/spi-nor/aspeed-smc.c                   | 746 +++++++++++++++++++++
 4 files changed, 845 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/aspeed-smc.txt
 create mode 100644 drivers/mtd/spi-nor/aspeed-smc.c

diff --git a/Documentation/devicetree/bindings/mtd/aspeed-smc.txt b/Documentation/devicetree/bindings/mtd/aspeed-smc.txt
new file mode 100644
index 000000000000..f6bfa7761205
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/aspeed-smc.txt
@@ -0,0 +1,86 @@
+* Aspeed Static Memory controller
+* Aspeed SPI Flash Controller
+
+The Static memory controller in the ast2400 supports 5 chip selects each
+can be attached to NAND, parallel NOR, or SPI NOR attached flash.  The
+Firmware Memory Controller in the ast2500 supports 3 chip selects, two of
+which are always in SPI-NOR mode and the third can be SPI-NOR or parallel
+flash.  The SPI flash controller in the ast2400 supports one of 2 chip
+selects selected by pinmux.  The two SPI flash controllers in the ast2500
+each support two chip selects.
+
+Required properties:
+  - compatible : Should be one of
+	"aspeed,ast2400-fmc" for the AST2400 Static Memory Controller
+	"aspeed,ast2400-smc" for the AST2400 SPI Flash Controller
+	"aspeed,ast2500-fmc" for the AST2500 Firmware SPI Memory Controller
+	"aspeed,ast2500-smc" for the AST2500 SPI Flash Controllers
+  - reg : the first contains the control register location and length,
+          the second contains the memory window mapping address and length
+  - clocks : The APB clock input to the controller
+  - #address-cells : must be 1 corresponding to chip select child binding
+  - #size-cells : must be 0 corresponding to chip select child binding
+
+Optional properties:
+  - aspeed,fmc-has-dma : controller supports DMA transfers
+  - interrupts : Should contain the interrupt for the dma device if an fmc
+
+Child node required properties:
+  - reg : must contain chip select number in first cell of address, must
+	  be 1 tuple long
+  - compatible : may contain "vendor,part", must include "jedec,spi-nor"
+		when attached to SPI flash (see spi-nor.txt binding).
+
+Child node optional properties:
+  - label           - (optional) name to assign to mtd, default os assigned
+
+Child node optional properties for SPI mode (may be ignored):
+  - spi-max-frequency - (optional) max frequency of spi bus
+  - spi-cpol        - (optional) Empty property indicating device requires
+			inverse clock polarity (CPOL) mode (boolean)
+  - spi-cpha        - (optional) Empty property indicating device requires
+			shifted clock phase (CPHA) mode (boolean)
+  - spi-tx-bus-width - (optional) The bus width(number of data wires) that
+                        used for MOSI. Defaults to 1 if not present.
+  - spi-rx-bus-width - (optional) The bus width(number of data wires) that
+                        used for MOSI. Defaults to 1 if not present.
+
+Grandchild node optional properties:
+ - see mtd/partiton.txt for partitioning bindings and mtd naming
+
+
+Example:
+
+fmc: fmc@1e620000 {
+	compatible = "aspeed,ast2400-fmc";
+	reg = < 0x1e620000 0x94
+		0x20000000 0x02000000
+		0x22000000 0x02000000 >;
+	#address-cells = <1>;
+	#size-cells = <0>;
+	flash@0 {
+		reg = < 0 >;
+		compatible = "jedec,spi-nor" ;
+		label = "bmc";
+		/* spi-max-frequency = <>; */
+		/* m25p,fast-read; */
+		#address-cells = <1>;
+		#size-cells = <1>;
+		boot@0 {
+			label = "boot-loader";
+			reg = < 0 0x8000 >
+		}
+		image@8000 {
+			label = "kernel-image";
+			reg = < 0x8000 0x1f8000 >
+		}
+	};
+	flash@1 {
+		reg = < 1 >;
+		compatible = "jedec,spi-nor" ;
+		label = "alt";
+		/* spi-max-frequency = <>; */
+		status = "fail";
+		/* m25p,fast-read; */
+	};
+};
diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
index 4a682ee0f632..96148600fdab 100644
--- a/drivers/mtd/spi-nor/Kconfig
+++ b/drivers/mtd/spi-nor/Kconfig
@@ -76,4 +76,16 @@ config SPI_NXP_SPIFI
 	  Flash. Enable this option if you have a device with a SPIFI
 	  controller and want to access the Flash as a mtd device.
 
+config ASPEED_FLASH_SPI
+	tristate "Aspeed flash controllers in SPI mode"
+	depends on HAS_IOMEM && OF
+	depends on ARCH_ASPEED || COMPILE_TEST
+	# IO_SPACE_LIMIT must be equivalent to (~0UL)
+	depends on !NEED_MACH_IO_H
+	help
+	  This enables support for the New Static Memory Controller
+	  (FMC) in the Aspeed SoCs (AST2400 and AST2500) when attached
+	  to SPI nor chips, and support for the SPI Memory controller
+	  (SPI) for the BIOS.
+
 endif # MTD_SPI_NOR
diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
index 121695e83542..c3174ebc45c2 100644
--- a/drivers/mtd/spi-nor/Makefile
+++ b/drivers/mtd/spi-nor/Makefile
@@ -4,4 +4,5 @@ obj-$(CONFIG_SPI_CADENCE_QUADSPI)	+= cadence-quadspi.o
 obj-$(CONFIG_SPI_FSL_QUADSPI)	+= fsl-quadspi.o
 obj-$(CONFIG_SPI_HISI_SFC)	+= hisi-sfc.o
 obj-$(CONFIG_MTD_MT81xx_NOR)    += mtk-quadspi.o
+obj-$(CONFIG_ASPEED_FLASH_SPI)	+= aspeed-smc.o
 obj-$(CONFIG_SPI_NXP_SPIFI)	+= nxp-spifi.o
diff --git a/drivers/mtd/spi-nor/aspeed-smc.c b/drivers/mtd/spi-nor/aspeed-smc.c
new file mode 100644
index 000000000000..a1b9d6711f73
--- /dev/null
+++ b/drivers/mtd/spi-nor/aspeed-smc.c
@@ -0,0 +1,746 @@
+/*
+ * ASPEED Static Memory Controller driver
+ *
+ * Copyright (c) 2015-2016, IBM Corporation.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include <linux/bug.h>
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/partitions.h>
+#include <linux/mtd/spi-nor.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/sysfs.h>
+
+#define DEVICE_NAME	"aspeed-smc"
+
+/*
+ * In user mode all data bytes read or written to the chip decode address
+ * range are transferred to or from the SPI bus. The range is treated as a
+ * fifo of arbitratry 1, 2, or 4 byte width but each write has to be aligned
+ * to its size.  The address within the multiple 8kB range is ignored when
+ * sending bytes to the SPI bus.
+ *
+ * On the arm architecture, as of Linux version 4.3, memcpy_fromio and
+ * memcpy_toio on little endian targets use the optimized memcpy routines
+ * that were designed for well behavied memory storage.  These routines
+ * have a stutter if the source and destination are not both word aligned,
+ * once with a duplicate access to the source after aligning to the
+ * destination to a word boundary, and again with a duplicate access to
+ * the source when the final byte count is not word aligned.
+ *
+ * When writing or reading the fifo this stutter discards data or sends
+ * too much data to the fifo and can not be used by this driver.
+ *
+ * While the low level io string routines that implement the insl family do
+ * the desired accesses and memory increments, the cross architecture io
+ * macros make them essentially impossible to use on a memory mapped address
+ * instead of a a token from the call to iomap of an io port.
+ *
+ * These fifo routines use readl and friends to a constant io port and update
+ * the memory buffer pointer and count via explicit code. The final updates
+ * to len are optimistically suppressed.
+ */
+
+static void aspeed_smc_from_fifo(void *buf, const void __iomem *iop, size_t len)
+{
+	if (!len)
+		return;
+
+	/* Expect a 4 byte input port.  Otherwise just read bytes. */
+	if (unlikely((unsigned long)iop & 3)) {
+		while (len--) {
+			*(u8 *)buf = readb(iop);
+			buf++;
+		}
+	}
+
+	/* Align target to word: first byte then half word */
+	if ((unsigned long)buf & 1) {
+		*(u8 *)buf = readb(iop);
+		buf++;
+		len--;
+	}
+	if (((unsigned long)buf & 2) && (len >= 2)) {
+		*(u16 *)buf = readw(iop);
+		buf += 2;
+		len -= 2;
+	}
+
+	/* Transfer words, then remaining halfword and remaining byte */
+	while (len >= 4) {
+		*(u32 *)buf = readl(iop);
+		buf += 4;
+		len -= 4;
+	}
+	if (len & 2) {
+		*(u16 *)buf = readw(iop);
+		buf += 2;
+	}
+	if (len & 1)
+		*(u8 *)buf = readb(iop);
+}
+
+static void aspeed_smc_to_fifo(void __iomem *iop, const void *buf, size_t len)
+{
+	if (!len)
+		return;
+
+	/* Expect a 4 byte output port.  Otherwise just write bytes. */
+	if ((unsigned long)iop & 3) {
+		while (len--) {
+			writeb(*(u8 *)buf, iop);
+			buf++;
+		}
+		return;
+	}
+
+	/* Align target to word: first byte then half word */
+	if ((unsigned long)buf & 1) {
+		writeb(*(u8 *)buf, iop);
+		buf++;
+		len--;
+	}
+	if (((unsigned long)buf & 2) && (len >= 2)) {
+		writew(*(u16 *)buf, iop);
+		buf += 2;
+		len -= 2;
+	}
+
+	/* Transfer words, then remaining halfword and remaining byte */
+	while (len >= 4) {
+		writel(*(u32 *)buf, iop);
+		buf += 4;
+		len -= 4;
+	}
+	if (len & 2) {
+		writew(*(u16 *)buf, iop);
+		buf += 2;
+	}
+	if (len & 1)
+		writeb(*(u8 *)buf, iop);
+}
+
+enum smc_flash_type {
+	smc_type_nor = 0,	/* controller connected to nor flash */
+	smc_type_nand = 1,	/* controller connected to nand flash */
+	smc_type_spi = 2,	/* controller connected to spi flash */
+};
+
+struct aspeed_smc_info {
+	u32 maxsize;		/* maximum size of 1 chip window */
+	u8 nce;			/* number of chip enables */
+	u8 maxwidth;		/* max width of spi bus */
+	bool hastype;		/* flash type field exists in cfg reg */
+	u8 we0;			/* shift for write enable bit for ce 0 */
+	u8 ctl0;		/* offset in regs of ctl for ce 0 */
+	u8 time;		/* offset in regs of timing */
+	u8 misc;		/* offset in regs of misc settings */
+};
+
+static struct aspeed_smc_info fmc_2400_info = {
+	.maxsize = 64 * 1024 * 1024,
+	.nce = 5,
+	.maxwidth = 4,
+	.hastype = true,
+	.we0 = 16,
+	.ctl0 = 0x10,
+	.time = 0x94,
+	.misc = 0x54,
+};
+
+static struct aspeed_smc_info smc_2400_info = {
+	.maxsize = 64 * 1024 * 1024,
+	.nce = 1,
+	.maxwidth = 2,
+	.hastype = false,
+	.we0 = 0,
+	.ctl0 = 0x04,
+	.time = 0x14,
+	.misc = 0x10,
+};
+
+static struct aspeed_smc_info fmc_2500_info = {
+	.maxsize = 256 * 1024 * 1024,
+	.nce = 3,
+	.maxwidth = 2,
+	.hastype = true,
+	.we0 = 16,
+	.ctl0 = 0x10,
+	.time = 0x94,
+	.misc = 0x54,
+};
+
+static struct aspeed_smc_info smc_2500_info = {
+	.maxsize = 128 * 1024 * 1024,
+	.nce = 2,
+	.maxwidth = 2,
+	.hastype = false,
+	.we0 = 16,
+	.ctl0 = 0x10,
+	.time = 0x94,
+	.misc = 0x54,
+};
+
+enum smc_ctl_reg_value {
+	smc_base,		/* base value without mode for other commands */
+	smc_read,		/* command reg for (maybe fast) reads */
+	smc_write,		/* command reg for writes with timings */
+	smc_num_ctl_reg_values	/* last value to get count of commands */
+};
+
+struct aspeed_smc_controller;
+
+struct aspeed_smc_chip {
+	int cs;
+	struct aspeed_smc_controller *controller;
+	__le32 __iomem *ctl;			/* control register */
+	void __iomem *base;			/* base of chip window */
+	__le32 ctl_val[smc_num_ctl_reg_values];	/* controls with timing */
+	enum smc_flash_type type;		/* what type of flash */
+	struct spi_nor nor;
+};
+
+struct aspeed_smc_controller {
+	struct device *dev;
+
+	struct mutex mutex;			/* controller access mutex */
+	const struct aspeed_smc_info *info;	/* type info of controller */
+	void __iomem *regs;			/* controller registers */
+	void __iomem *windows;			/* per-chip windows resource */
+
+	struct aspeed_smc_chip *chips[0];	/* pointers to attached chips */
+};
+
+/*
+ * FMC Type setting Register
+ *   or
+ * SPI Flash Configuration Register
+ */
+#define CONFIG_REG			0x0
+
+/*
+ * CE Control Register
+ */
+#define CE_CONTROL_REG			0x4
+#define    CE0_CONTROL_EXTENDED		    BIT(0)
+
+/* CE0 Control Register (depends on the controller type) */
+#define CONTROL_SPI_AAF_MODE BIT(31)
+#define CONTROL_SPI_IO_MODE_MASK GENMASK(30, 28)
+#define CONTROL_SPI_IO_DUAL_DATA BIT(29)
+#define CONTROL_SPI_IO_DUAL_ADDR_DATA (BIT(29) | BIT(28))
+#define CONTROL_SPI_IO_QUAD_DATA BIT(30)
+#define CONTROL_SPI_IO_QUAD_ADDR_DATA (BIT(30) | BIT(28))
+#define CONTROL_SPI_CE_INACTIVE_SHIFT 24
+#define CONTROL_SPI_CE_INACTIVE_MASK GENMASK(27, CONTROL_SPI_CE_INACTIVE_SHIFT)
+/* 0 = 16T ... 15 = 1T   T=HCLK */
+#define CONTROL_SPI_COMMAND_SHIFT 16
+#define CONTROL_SPI_DUMMY_CYCLE_COMMAND_OUTPUT BIT(15)
+#define CONTROL_SPI_IO_DUMMY_CYCLES_HI BIT(14)
+#define CONTROL_SPI_IO_DUMMY_CYCLES_HI_SHIFT (14 - 2)
+#define CONTROL_SPI_IO_ADDRESS_4B BIT(13) /* 2400-smc */
+#define CONTROL_SPI_CLK_DIV4 BIT(13) /* FMC, 2500 */
+#define CONTROL_SPI_RW_MERGE BIT(12)
+#define CONTROL_SPI_IO_DUMMY_CYCLES_LO_SHIFT 6
+#define CONTROL_SPI_IO_DUMMY_CYCLES_LO GENMASK(7, \
+				       CONTROL_SPI_IO_DUMMY_CYCLES_LO_SHIFT)
+#define CONTROL_SPI_IO_DUMMY_CYCLES_MASK (CONTROL_SPI_IO_DUMMY_CYCLES_HI | \
+					  CONTROL_SPI_IO_DUMMY_CYCLES_LO)
+#define CONTROL_SPI_CLOCK_FREQ_SEL_SHIFT 8
+#define CONTROL_SPI_CLOCK_FREQ_SEL_MASK GENMASK(11, \
+					CONTROL_SPI_CLOCK_FREQ_SEL_SHIFT)
+#define CONTROL_SPI_LSB_FIRST BIT(5)
+#define CONTROL_SPI_CLOCK_MODE_3 BIT(4)
+#define CONTROL_SPI_IN_DUAL_DATA BIT(3)
+#define CONTROL_SPI_CE_STOP_ACTIVE_CONTROL BIT(2)
+#define CONTROL_SPI_COMMAND_MODE_MASK GENMASK(1, 0)
+#define CONTROL_SPI_COMMAND_MODE_NORMAL (0)
+#define CONTROL_SPI_COMMAND_MODE_FREAD (1)
+#define CONTROL_SPI_COMMAND_MODE_WRITE (2)
+#define CONTROL_SPI_COMMAND_MODE_USER (3)
+
+#define CONTROL_SPI_KEEP_MASK (CONTROL_SPI_AAF_MODE | \
+	CONTROL_SPI_CE_INACTIVE_MASK | CONTROL_SPI_CLK_DIV4 | \
+	CONTROL_SPI_IO_DUMMY_CYCLES_MASK | CONTROL_SPI_CLOCK_FREQ_SEL_MASK | \
+	CONTROL_SPI_LSB_FIRST | CONTROL_SPI_CLOCK_MODE_3)
+
+/* Segment Address Registers */
+#define SEGMENT_ADDR_REG0		0x30
+#define     SEGMENT_ADDR_START(_r)	    ((((_r) >> 16) & 0xFF) << 23)
+#define     SEGMENT_ADDR_END(_r)	    ((((_r) >> 24) & 0xFF) << 23)
+
+static u32 spi_control_fill_opcode(u8 opcode)
+{
+	return ((u32)(opcode)) << CONTROL_SPI_COMMAND_SHIFT;
+}
+
+static inline u32 aspeed_smc_chip_write_bit(struct aspeed_smc_chip *chip)
+{
+	return ((u32)1 << (chip->controller->info->we0 + chip->cs));
+}
+
+static void aspeed_smc_chip_check_config(struct aspeed_smc_chip *chip)
+{
+	struct aspeed_smc_controller *controller = chip->controller;
+	u32 reg;
+
+	reg = readl(controller->regs + CONFIG_REG);
+
+	if (!(reg & aspeed_smc_chip_write_bit(chip))) {
+		dev_dbg(controller->dev,
+			"config write is not set ! @%p: 0x%08x\n",
+			controller->regs + CONFIG_REG, reg);
+		reg |= aspeed_smc_chip_write_bit(chip);
+		writel(reg, controller->regs + CONFIG_REG);
+	}
+}
+
+static void aspeed_smc_start_user(struct spi_nor *nor)
+{
+	struct aspeed_smc_chip *chip = nor->priv;
+	u32 ctl = chip->ctl_val[smc_base];
+
+	/*
+	 * When the chip is controlled in user mode, we need write
+	 * access to send the opcodes to it. So check the config.
+	 */
+	aspeed_smc_chip_check_config(chip);
+
+	ctl |= CONTROL_SPI_COMMAND_MODE_USER |
+		CONTROL_SPI_CE_STOP_ACTIVE_CONTROL;
+	writel(ctl, chip->ctl);
+
+	ctl &= ~CONTROL_SPI_CE_STOP_ACTIVE_CONTROL;
+	writel(ctl, chip->ctl);
+}
+
+static void aspeed_smc_stop_user(struct spi_nor *nor)
+{
+	struct aspeed_smc_chip *chip = nor->priv;
+
+	u32 ctl = chip->ctl_val[smc_read];
+	u32 ctl2 = ctl | CONTROL_SPI_COMMAND_MODE_USER |
+		CONTROL_SPI_CE_STOP_ACTIVE_CONTROL;
+
+	writel(ctl2, chip->ctl);	/* stop user CE control */
+	writel(ctl, chip->ctl);		/* default to fread or read */
+}
+
+static int aspeed_smc_read_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
+{
+	struct aspeed_smc_chip *chip = nor->priv;
+
+	mutex_lock(&chip->controller->mutex);
+
+	aspeed_smc_start_user(nor);
+	aspeed_smc_to_fifo(chip->base, &opcode, 1);
+	aspeed_smc_from_fifo(buf, chip->base, len);
+	aspeed_smc_stop_user(nor);
+
+	mutex_unlock(&chip->controller->mutex);
+
+	return 0;
+}
+
+static int aspeed_smc_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf,
+				int len)
+{
+	struct aspeed_smc_chip *chip = nor->priv;
+
+	mutex_lock(&chip->controller->mutex);
+
+	aspeed_smc_start_user(nor);
+	aspeed_smc_to_fifo(chip->base, &opcode, 1);
+	aspeed_smc_to_fifo(chip->base, buf, len);
+	aspeed_smc_stop_user(nor);
+
+	mutex_unlock(&chip->controller->mutex);
+
+	return 0;
+}
+
+static void aspeed_smc_send_cmd_addr(struct spi_nor *nor, u8 cmd, u32 addr)
+{
+	struct aspeed_smc_chip *chip = nor->priv;
+	__be32 temp;
+	u32 cmdaddr;
+
+	switch (nor->addr_width) {
+	default:
+		WARN_ONCE(1, "Unexpected address width %u, defaulting to 3\n",
+			  nor->addr_width);
+		/* FALLTHROUGH */
+	case 3:
+		cmdaddr = addr & 0xFFFFFF;
+
+		cmdaddr |= (u32)cmd << 24;
+
+		temp = cpu_to_be32(cmdaddr);
+		aspeed_smc_to_fifo(chip->base, &temp, 4);
+		break;
+	case 4:
+		temp = cpu_to_be32(addr);
+		aspeed_smc_to_fifo(chip->base, &cmd, 1);
+		aspeed_smc_to_fifo(chip->base, &temp, 4);
+		break;
+	}
+}
+
+static ssize_t aspeed_smc_read_user(struct spi_nor *nor, loff_t from, size_t len,
+				    u_char *read_buf)
+{
+	struct aspeed_smc_chip *chip = nor->priv;
+
+	mutex_lock(&chip->controller->mutex);
+
+	aspeed_smc_start_user(nor);
+	aspeed_smc_send_cmd_addr(nor, nor->read_opcode, from);
+	aspeed_smc_from_fifo(read_buf, chip->base, len);
+	aspeed_smc_stop_user(nor);
+
+	mutex_unlock(&chip->controller->mutex);
+
+	return len;
+}
+
+static ssize_t aspeed_smc_write_user(struct spi_nor *nor, loff_t to, size_t len,
+				     const u_char *write_buf)
+{
+	struct aspeed_smc_chip *chip = nor->priv;
+
+	mutex_lock(&chip->controller->mutex);
+
+	aspeed_smc_start_user(nor);
+	aspeed_smc_send_cmd_addr(nor, nor->program_opcode, to);
+	aspeed_smc_to_fifo(chip->base, write_buf, len);
+	aspeed_smc_stop_user(nor);
+
+	mutex_unlock(&chip->controller->mutex);
+
+	return len;
+}
+
+static int aspeed_smc_remove(struct platform_device *dev)
+{
+	struct aspeed_smc_chip *chip;
+	struct aspeed_smc_controller *controller = platform_get_drvdata(dev);
+	int n;
+
+	for (n = 0; n < controller->info->nce; n++) {
+		chip = controller->chips[n];
+		if (chip)
+			mtd_device_unregister(&chip->nor.mtd);
+	}
+
+	return 0;
+}
+
+const struct of_device_id aspeed_smc_matches[] = {
+	{ .compatible = "aspeed,ast2400-fmc", .data = &fmc_2400_info },
+	{ .compatible = "aspeed,ast2400-smc", .data = &smc_2400_info },
+	{ .compatible = "aspeed,ast2500-fmc", .data = &fmc_2500_info },
+	{ .compatible = "aspeed,ast2500-smc", .data = &smc_2500_info },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, aspeed_smc_matches);
+
+static struct platform_device *
+of_platform_device_create_or_find(struct device_node *child,
+				  struct device *parent)
+{
+	struct platform_device *cdev;
+
+	cdev = of_platform_device_create(child, NULL, parent);
+	if (!cdev)
+		cdev = of_find_device_by_node(child);
+	return cdev;
+}
+
+static void __iomem *window_start(struct aspeed_smc_controller *controller,
+				  struct resource *r, unsigned int n)
+{
+	u32 offset = 0;
+	u32 reg;
+
+	if (controller->info->nce > 1) {
+		reg = readl(controller->regs + SEGMENT_ADDR_REG0 + n * 4);
+
+		if (SEGMENT_ADDR_START(reg) >= SEGMENT_ADDR_END(reg))
+			return NULL;
+
+		offset = SEGMENT_ADDR_START(reg) - r->start;
+	}
+
+	return controller->windows + offset;
+}
+
+static void aspeed_smc_chip_enable_write(struct aspeed_smc_chip *chip)
+{
+	struct aspeed_smc_controller *controller = chip->controller;
+	u32 reg;
+
+	reg = readl(controller->regs + CONFIG_REG);
+	dev_dbg(controller->dev, "config reg @%p: 0x%08x\n",
+		controller->regs + CONFIG_REG, reg);
+
+	reg |= aspeed_smc_chip_write_bit(chip);
+	writel(reg, controller->regs + CONFIG_REG);
+}
+
+static void aspeed_smc_chip_set_type(struct aspeed_smc_chip *chip, int type)
+{
+	struct aspeed_smc_controller *controller = chip->controller;
+	u32 reg;
+
+	reg = readl(controller->regs + CONFIG_REG);
+	dev_dbg(controller->dev, "config reg @%p: 0x%08x\n",
+		controller->regs + CONFIG_REG, reg);
+
+	chip->type = type;
+
+	reg &= ~(3 << (chip->cs * 2));
+	reg |= chip->type << (chip->cs * 2);
+	writel(reg, controller->regs + CONFIG_REG);
+}
+
+static int aspeed_smc_chip_setup_init(struct aspeed_smc_chip *chip,
+				      struct resource *r)
+{
+	struct aspeed_smc_controller *controller = chip->controller;
+	const struct aspeed_smc_info *info = controller->info;
+	u32 reg;
+
+	/*
+	 * Always turn on the write enable bit to allow opcodes to be
+	 * sent in user mode.
+	 */
+	aspeed_smc_chip_enable_write(chip);
+
+	/* The driver only supports SPI type flash for the moment */
+	if (info->hastype)
+		aspeed_smc_chip_set_type(chip, smc_type_spi);
+
+	/*
+	 * Configure chip base address in memory
+	 */
+	chip->base = window_start(controller, r, chip->cs);
+	if (!chip->base) {
+		dev_warn(chip->nor.dev, "CE segment window closed.\n");
+		return -1;
+	}
+
+	/*
+	 * Read the existing control register to get basic values.
+	 *
+	 * XXX This register probably needs more sanitation.
+	 *
+	 * Do we need support for mode 3 vs mode 0 clock phasing?
+	 */
+	reg = readl(chip->ctl);
+	dev_dbg(controller->dev, "control register: %08x\n", reg);
+
+	if ((reg & CONTROL_SPI_KEEP_MASK) != reg) {
+		chip->ctl_val[smc_base] = reg & CONTROL_SPI_KEEP_MASK;
+		dev_info(controller->dev,
+			 "control register changed to: %08x\n",
+			 chip->ctl_val[smc_base]);
+	}
+
+	/*
+	 * 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_SPI_COMMAND_MODE_MASK) ==
+	    CONTROL_SPI_COMMAND_MODE_NORMAL)
+		chip->ctl_val[smc_read] = reg;
+	else
+		chip->ctl_val[smc_read] = chip->ctl_val[smc_base] |
+			CONTROL_SPI_COMMAND_MODE_NORMAL;
+
+	return 0;
+}
+
+static void aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip)
+{
+	struct aspeed_smc_controller *controller = chip->controller;
+	u32 reg;
+
+	/*
+	 * Set 4 byte mode in the chip controller register and also in
+	 * controller config register. The BMC flash controller is
+	 * strapped by hardware, or autodetected, but the SPI flash
+	 * controller of the AST2500 still needs to be set.
+	 */
+	if (chip->nor.mtd.size > SZ_16M) {
+		chip->ctl_val[smc_base] |= CONTROL_SPI_IO_ADDRESS_4B;
+
+		/*
+		 * The SPI flash controller of the AST2400 does not
+		 * have such a setting.
+		 */
+		if (chip->controller->info == &smc_2500_info) {
+			reg = readl(controller->regs + CE_CONTROL_REG);
+			reg |= 1 << chip->cs;
+			writel(reg, controller->regs + CE_CONTROL_REG);
+		}
+	}
+
+	chip->ctl_val[smc_write] = chip->ctl_val[smc_base] |
+		spi_control_fill_opcode(chip->nor.program_opcode) |
+		CONTROL_SPI_COMMAND_MODE_WRITE;
+
+	/*
+	 * XXX TODO
+	 * Enable fast read mode as required here.
+	 * Adjust clocks if fast read and write are supported.
+	 * Interpret spi-nor flags to adjust controller settings.
+	 * Check if resource size big enough for detected chip and
+	 * add support assisted (normal or fast-) read and dma.
+	 */
+}
+
+static int aspeed_smc_probe(struct platform_device *pdev)
+{
+	struct aspeed_smc_controller *controller;
+	const struct of_device_id *match;
+	const struct aspeed_smc_info *info;
+	struct resource *r;
+	struct device_node *child;
+	int err = 0;
+	unsigned int n;
+
+	match = of_match_device(aspeed_smc_matches, &pdev->dev);
+	if (!match || !match->data)
+		return -ENODEV;
+	info = match->data;
+
+	controller = devm_kzalloc(&pdev->dev, sizeof(*controller) +
+		info->nce * sizeof(controller->chips[0]), GFP_KERNEL);
+	if (!controller)
+		return -ENOMEM;
+	controller->info = info;
+
+	mutex_init(&controller->mutex);
+	platform_set_drvdata(pdev, controller);
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	controller->regs = devm_ioremap_resource(&pdev->dev, r);
+	if (IS_ERR(controller->regs))
+		return PTR_ERR(controller->regs);
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	controller->windows = devm_ioremap_resource(&pdev->dev, r);
+	if (IS_ERR(controller->windows))
+		return PTR_ERR(controller->windows);
+
+	controller->dev = &pdev->dev;
+
+	/* The pinmux or bootloader will disable the legacy mode controller */
+
+	/*
+	 * XXX Need to add arbitration to the SMC (BIOS) controller if access
+	 * is shared by the host.
+	 */
+	for_each_available_child_of_node(controller->dev->of_node, child) {
+		struct platform_device *cdev;
+		struct aspeed_smc_chip *chip;
+
+		/* This version does not support nand or nor flash devices. */
+		if (!of_device_is_compatible(child, "jedec,spi-nor"))
+			continue;
+
+		/*
+		 * create a platform device from the of node.  If the device
+		 * already was created (eg from a prior bind/unbind cycle)
+		 * reuse it.
+		 *
+		 * The creating the device node for the child here allows its
+		 * use for error reporting via dev_err below.
+		 */
+		cdev = of_platform_device_create_or_find(child,
+							 controller->dev);
+		if (!cdev)
+			continue;
+
+		err = of_property_read_u32(child, "reg", &n);
+		if (err == -EINVAL && info->nce == 1)
+			n = 0;
+		else if (err || n >= info->nce)
+			continue;
+		if (controller->chips[n]) {
+			dev_err(&cdev->dev,
+				"chip-id %u already in use in use by %s\n",
+				n, dev_name(controller->chips[n]->nor.dev));
+			continue;
+		}
+
+		chip = devm_kzalloc(controller->dev, sizeof(*chip), GFP_KERNEL);
+		if (!chip)
+			continue;
+		chip->controller = controller;
+		chip->ctl = controller->regs + info->ctl0 + n * 4;
+		chip->cs = n;
+
+		chip->nor.dev = &cdev->dev;
+		chip->nor.priv = chip;
+		spi_nor_set_flash_node(&chip->nor, child);
+		chip->nor.mtd.name = of_get_property(child, "label", NULL);
+		chip->nor.read = aspeed_smc_read_user;
+		chip->nor.write = aspeed_smc_write_user;
+		chip->nor.read_reg = aspeed_smc_read_reg;
+		chip->nor.write_reg = aspeed_smc_write_reg;
+
+		aspeed_smc_chip_setup_init(chip, r);
+
+		/*
+		 * XXX Add support for SPI_NOR_QUAD and SPI_NOR_DUAL attach
+		 * when board support is present as determined by of property.
+		 */
+		err = spi_nor_scan(&chip->nor, NULL, SPI_NOR_NORMAL);
+		if (err)
+			continue;
+
+		aspeed_smc_chip_setup_finish(chip);
+
+		err = mtd_device_register(&chip->nor.mtd, NULL, 0);
+		if (err)
+			continue;
+		controller->chips[n] = chip;
+	}
+
+	/* Were any children registered? */
+	for (n = 0; n < info->nce; n++)
+		if (controller->chips[n])
+			break;
+
+	if (n == info->nce)
+		return -ENODEV;
+
+	return 0;
+}
+
+static struct platform_driver aspeed_smc_driver = {
+	.probe = aspeed_smc_probe,
+	.remove = aspeed_smc_remove,
+	.driver = {
+		.name = DEVICE_NAME,
+		.of_match_table = aspeed_smc_matches,
+	}
+};
+
+module_platform_driver(aspeed_smc_driver);
+
+MODULE_DESCRIPTION("ASPEED Static Memory Controller Driver");
+MODULE_AUTHOR("Milton Miller");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] mtd/spi-nor: Add SPI memory controllers for Aspeed SoCs
  2016-10-17 16:57 ` Cédric Le Goater
@ 2016-10-18 16:39     ` Rob Herring
  -1 siblings, 0 replies; 12+ messages in thread
From: Rob Herring @ 2016-10-18 16:39 UTC (permalink / raw)
  To: Cédric Le Goater
  Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, David Woodhouse,
	Brian Norris, devicetree-u79uwXL29TY76Z2rM5mHXA, Joel Stanley,
	Milton Miller

On Mon, Oct 17, 2016 at 06:57:07PM +0200, Cédric Le Goater wrote:
> This driver adds mtd support for spi-nor attached to either or both of
> the Firmware Memory Controller or the SPI Flash Controller (AST2400
> only).
> 
> The SMC controllers on the Aspeed AST2500 SoC are very similar to the
> ones found on the AST2400. The differences are on the number of
> supported flash modules and their default mappings in the SoC address
> space.
> 
> The Aspeed AST2500 has one SPI controller for the BMC firmware and two
> for the host firmware. All controllers have now the same set of
> registers compatible with the AST2400 FMC controller and the legacy
> 'SMC' controller is fully gone.
> 
> Based on previous work from Milton D. Miller II <miltonm-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
> 
> Signed-off-by: Cédric Le Goater <clg-Bxea+6Xhats@public.gmane.org>
> ---
>  .../devicetree/bindings/mtd/aspeed-smc.txt         |  86 +++
>  drivers/mtd/spi-nor/Kconfig                        |  12 +
>  drivers/mtd/spi-nor/Makefile                       |   1 +
>  drivers/mtd/spi-nor/aspeed-smc.c                   | 746 +++++++++++++++++++++
>  4 files changed, 845 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/aspeed-smc.txt
>  create mode 100644 drivers/mtd/spi-nor/aspeed-smc.c
> 
> diff --git a/Documentation/devicetree/bindings/mtd/aspeed-smc.txt b/Documentation/devicetree/bindings/mtd/aspeed-smc.txt
> new file mode 100644
> index 000000000000..f6bfa7761205
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/aspeed-smc.txt
> @@ -0,0 +1,86 @@
> +* Aspeed Static Memory controller
> +* Aspeed SPI Flash Controller
> +
> +The Static memory controller in the ast2400 supports 5 chip selects each
> +can be attached to NAND, parallel NOR, or SPI NOR attached flash.  The
> +Firmware Memory Controller in the ast2500 supports 3 chip selects, two of
> +which are always in SPI-NOR mode and the third can be SPI-NOR or parallel
> +flash.  The SPI flash controller in the ast2400 supports one of 2 chip
> +selects selected by pinmux.  The two SPI flash controllers in the ast2500
> +each support two chip selects.
> +
> +Required properties:
> +  - compatible : Should be one of
> +	"aspeed,ast2400-fmc" for the AST2400 Static Memory Controller
> +	"aspeed,ast2400-smc" for the AST2400 SPI Flash Controller
> +	"aspeed,ast2500-fmc" for the AST2500 Firmware SPI Memory Controller
> +	"aspeed,ast2500-smc" for the AST2500 SPI Flash Controllers
> +  - reg : the first contains the control register location and length,
> +          the second contains the memory window mapping address and length
> +  - clocks : The APB clock input to the controller
> +  - #address-cells : must be 1 corresponding to chip select child binding
> +  - #size-cells : must be 0 corresponding to chip select child binding
> +
> +Optional properties:
> +  - aspeed,fmc-has-dma : controller supports DMA transfers

The compatible should imply this.

> +  - interrupts : Should contain the interrupt for the dma device if an fmc
> +
> +Child node required properties:

Need to say what the child nodes are.

> +  - reg : must contain chip select number in first cell of address, must
> +	  be 1 tuple long
> +  - compatible : may contain "vendor,part", must include "jedec,spi-nor"
> +		when attached to SPI flash (see spi-nor.txt binding).
> +
> +Child node optional properties:
> +  - label           - (optional) name to assign to mtd, default os assigned
> +
> +Child node optional properties for SPI mode (may be ignored):
> +  - spi-max-frequency - (optional) max frequency of spi bus
> +  - spi-cpol        - (optional) Empty property indicating device requires
> +			inverse clock polarity (CPOL) mode (boolean)
> +  - spi-cpha        - (optional) Empty property indicating device requires
> +			shifted clock phase (CPHA) mode (boolean)
> +  - spi-tx-bus-width - (optional) The bus width(number of data wires) that
> +                        used for MOSI. Defaults to 1 if not present.
> +  - spi-rx-bus-width - (optional) The bus width(number of data wires) that
> +                        used for MOSI. Defaults to 1 if not present.
> +
> +Grandchild node optional properties:
> + - see mtd/partiton.txt for partitioning bindings and mtd naming
> +
> +
> +Example:
> +
> +fmc: fmc@1e620000 {
> +	compatible = "aspeed,ast2400-fmc";
> +	reg = < 0x1e620000 0x94
> +		0x20000000 0x02000000
> +		0x22000000 0x02000000 >;
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	flash@0 {
> +		reg = < 0 >;
> +		compatible = "jedec,spi-nor" ;
> +		label = "bmc";

label isn't really a defined property here. Belongs in jedec,spi-nor 
binding if you want to add it.

> +		/* spi-max-frequency = <>; */
> +		/* m25p,fast-read; */
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		boot@0 {

Use the newer style with all partitions under a 'partitions' node.

> +			label = "boot-loader";
> +			reg = < 0 0x8000 >

Missing ';'

> +		}
> +		image@8000 {
> +			label = "kernel-image";
> +			reg = < 0x8000 0x1f8000 >

ditto

> +		}
> +	};
> +	flash@1 {
> +		reg = < 1 >;
> +		compatible = "jedec,spi-nor" ;
> +		label = "alt";
> +		/* spi-max-frequency = <>; */
> +		status = "fail";
> +		/* m25p,fast-read; */
> +	};
> +};
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] mtd/spi-nor: Add SPI memory controllers for Aspeed SoCs
@ 2016-10-18 16:39     ` Rob Herring
  0 siblings, 0 replies; 12+ messages in thread
From: Rob Herring @ 2016-10-18 16:39 UTC (permalink / raw)
  To: Cédric Le Goater
  Cc: linux-mtd, David Woodhouse, Brian Norris, devicetree,
	Joel Stanley, Milton Miller

On Mon, Oct 17, 2016 at 06:57:07PM +0200, Cédric Le Goater wrote:
> This driver adds mtd support for spi-nor attached to either or both of
> the Firmware Memory Controller or the SPI Flash Controller (AST2400
> only).
> 
> The SMC controllers on the Aspeed AST2500 SoC are very similar to the
> ones found on the AST2400. The differences are on the number of
> supported flash modules and their default mappings in the SoC address
> space.
> 
> The Aspeed AST2500 has one SPI controller for the BMC firmware and two
> for the host firmware. All controllers have now the same set of
> registers compatible with the AST2400 FMC controller and the legacy
> 'SMC' controller is fully gone.
> 
> Based on previous work from Milton D. Miller II <miltonm@us.ibm.com>
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  .../devicetree/bindings/mtd/aspeed-smc.txt         |  86 +++
>  drivers/mtd/spi-nor/Kconfig                        |  12 +
>  drivers/mtd/spi-nor/Makefile                       |   1 +
>  drivers/mtd/spi-nor/aspeed-smc.c                   | 746 +++++++++++++++++++++
>  4 files changed, 845 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/aspeed-smc.txt
>  create mode 100644 drivers/mtd/spi-nor/aspeed-smc.c
> 
> diff --git a/Documentation/devicetree/bindings/mtd/aspeed-smc.txt b/Documentation/devicetree/bindings/mtd/aspeed-smc.txt
> new file mode 100644
> index 000000000000..f6bfa7761205
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/aspeed-smc.txt
> @@ -0,0 +1,86 @@
> +* Aspeed Static Memory controller
> +* Aspeed SPI Flash Controller
> +
> +The Static memory controller in the ast2400 supports 5 chip selects each
> +can be attached to NAND, parallel NOR, or SPI NOR attached flash.  The
> +Firmware Memory Controller in the ast2500 supports 3 chip selects, two of
> +which are always in SPI-NOR mode and the third can be SPI-NOR or parallel
> +flash.  The SPI flash controller in the ast2400 supports one of 2 chip
> +selects selected by pinmux.  The two SPI flash controllers in the ast2500
> +each support two chip selects.
> +
> +Required properties:
> +  - compatible : Should be one of
> +	"aspeed,ast2400-fmc" for the AST2400 Static Memory Controller
> +	"aspeed,ast2400-smc" for the AST2400 SPI Flash Controller
> +	"aspeed,ast2500-fmc" for the AST2500 Firmware SPI Memory Controller
> +	"aspeed,ast2500-smc" for the AST2500 SPI Flash Controllers
> +  - reg : the first contains the control register location and length,
> +          the second contains the memory window mapping address and length
> +  - clocks : The APB clock input to the controller
> +  - #address-cells : must be 1 corresponding to chip select child binding
> +  - #size-cells : must be 0 corresponding to chip select child binding
> +
> +Optional properties:
> +  - aspeed,fmc-has-dma : controller supports DMA transfers

The compatible should imply this.

> +  - interrupts : Should contain the interrupt for the dma device if an fmc
> +
> +Child node required properties:

Need to say what the child nodes are.

> +  - reg : must contain chip select number in first cell of address, must
> +	  be 1 tuple long
> +  - compatible : may contain "vendor,part", must include "jedec,spi-nor"
> +		when attached to SPI flash (see spi-nor.txt binding).
> +
> +Child node optional properties:
> +  - label           - (optional) name to assign to mtd, default os assigned
> +
> +Child node optional properties for SPI mode (may be ignored):
> +  - spi-max-frequency - (optional) max frequency of spi bus
> +  - spi-cpol        - (optional) Empty property indicating device requires
> +			inverse clock polarity (CPOL) mode (boolean)
> +  - spi-cpha        - (optional) Empty property indicating device requires
> +			shifted clock phase (CPHA) mode (boolean)
> +  - spi-tx-bus-width - (optional) The bus width(number of data wires) that
> +                        used for MOSI. Defaults to 1 if not present.
> +  - spi-rx-bus-width - (optional) The bus width(number of data wires) that
> +                        used for MOSI. Defaults to 1 if not present.
> +
> +Grandchild node optional properties:
> + - see mtd/partiton.txt for partitioning bindings and mtd naming
> +
> +
> +Example:
> +
> +fmc: fmc@1e620000 {
> +	compatible = "aspeed,ast2400-fmc";
> +	reg = < 0x1e620000 0x94
> +		0x20000000 0x02000000
> +		0x22000000 0x02000000 >;
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	flash@0 {
> +		reg = < 0 >;
> +		compatible = "jedec,spi-nor" ;
> +		label = "bmc";

label isn't really a defined property here. Belongs in jedec,spi-nor 
binding if you want to add it.

> +		/* spi-max-frequency = <>; */
> +		/* m25p,fast-read; */
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		boot@0 {

Use the newer style with all partitions under a 'partitions' node.

> +			label = "boot-loader";
> +			reg = < 0 0x8000 >

Missing ';'

> +		}
> +		image@8000 {
> +			label = "kernel-image";
> +			reg = < 0x8000 0x1f8000 >

ditto

> +		}
> +	};
> +	flash@1 {
> +		reg = < 1 >;
> +		compatible = "jedec,spi-nor" ;
> +		label = "alt";
> +		/* spi-max-frequency = <>; */
> +		status = "fail";
> +		/* m25p,fast-read; */
> +	};
> +};

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] mtd/spi-nor: Add SPI memory controllers for Aspeed SoCs
  2016-10-17 16:57 ` Cédric Le Goater
  (?)
  (?)
@ 2016-10-19 17:06 ` Cyrille Pitchen
  2016-10-20  9:17   ` Cédric Le Goater
  -1 siblings, 1 reply; 12+ messages in thread
From: Cyrille Pitchen @ 2016-10-19 17:06 UTC (permalink / raw)
  To: linux-mtd

Hi Cédric,

Le 17/10/2016 à 18:57, Cédric Le Goater a écrit :
> This driver adds mtd support for spi-nor attached to either or both of
> the Firmware Memory Controller or the SPI Flash Controller (AST2400
> only).
> 
> The SMC controllers on the Aspeed AST2500 SoC are very similar to the
> ones found on the AST2400. The differences are on the number of
> supported flash modules and their default mappings in the SoC address
> space.
> 
> The Aspeed AST2500 has one SPI controller for the BMC firmware and two
> for the host firmware. All controllers have now the same set of
> registers compatible with the AST2400 FMC controller and the legacy
> 'SMC' controller is fully gone.
> 
> Based on previous work from Milton D. Miller II <miltonm@us.ibm.com>
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
[...]
> +
> +static void aspeed_smc_send_cmd_addr(struct spi_nor *nor, u8 cmd, u32 addr)
> +{
> +	struct aspeed_smc_chip *chip = nor->priv;
> +	__be32 temp;
> +	u32 cmdaddr;
> +
> +	switch (nor->addr_width) {
> +	default:
> +		WARN_ONCE(1, "Unexpected address width %u, defaulting to 3\n",
> +			  nor->addr_width);
> +		/* FALLTHROUGH */
> +	case 3:
> +		cmdaddr = addr & 0xFFFFFF;
> +
> +		cmdaddr |= (u32)cmd << 24;
> +
> +		temp = cpu_to_be32(cmdaddr);
> +		aspeed_smc_to_fifo(chip->base, &temp, 4);
> +		break;
> +	case 4:
> +		temp = cpu_to_be32(addr);
> +		aspeed_smc_to_fifo(chip->base, &cmd, 1);
> +		aspeed_smc_to_fifo(chip->base, &temp, 4);
> +		break;
> +	}
> +}
> +
> +static ssize_t aspeed_smc_read_user(struct spi_nor *nor, loff_t from, size_t len,
> +				    u_char *read_buf)
> +{
> +	struct aspeed_smc_chip *chip = nor->priv;
> +
> +	mutex_lock(&chip->controller->mutex);
> +
> +	aspeed_smc_start_user(nor);
> +	aspeed_smc_send_cmd_addr(nor, nor->read_opcode, from);

I guess the dummy cycles are missing between address cycles and data cycles!
Check nor->read_dummy ;)

Or maybe this controller can really only perform READ operations but no FAST
READ commands. Hence the "spi_nor_scan(&chip->nor, NULL, SPI_NOR_NORMAL)"
below.

> +	aspeed_smc_from_fifo(read_buf, chip->base, len);
> +	aspeed_smc_stop_user(nor);
> +
> +	mutex_unlock(&chip->controller->mutex);
> +
> +	return len;
> +}
[...]

Best regards,

Cyrille

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] mtd/spi-nor: Add SPI memory controllers for Aspeed SoCs
  2016-10-19 17:06 ` Cyrille Pitchen
@ 2016-10-20  9:17   ` Cédric Le Goater
  2016-10-20 14:33     ` Cyrille Pitchen
  0 siblings, 1 reply; 12+ messages in thread
From: Cédric Le Goater @ 2016-10-20  9:17 UTC (permalink / raw)
  To: Cyrille Pitchen, linux-mtd

Hello Cyrille,

Thanks for taking a look, some answers and more questions below.

On 10/19/2016 07:06 PM, Cyrille Pitchen wrote:
> Hi Cédric,
> 
> Le 17/10/2016 à 18:57, Cédric Le Goater a écrit :
>> This driver adds mtd support for spi-nor attached to either or both of
>> the Firmware Memory Controller or the SPI Flash Controller (AST2400
>> only).
>>
>> The SMC controllers on the Aspeed AST2500 SoC are very similar to the
>> ones found on the AST2400. The differences are on the number of
>> supported flash modules and their default mappings in the SoC address
>> space.
>>
>> The Aspeed AST2500 has one SPI controller for the BMC firmware and two
>> for the host firmware. All controllers have now the same set of
>> registers compatible with the AST2400 FMC controller and the legacy
>> 'SMC' controller is fully gone.
>>
>> Based on previous work from Milton D. Miller II <miltonm@us.ibm.com>
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
> [...]
>> +
>> +static void aspeed_smc_send_cmd_addr(struct spi_nor *nor, u8 cmd, u32 addr)
>> +{
>> +	struct aspeed_smc_chip *chip = nor->priv;
>> +	__be32 temp;
>> +	u32 cmdaddr;
>> +
>> +	switch (nor->addr_width) {
>> +	default:
>> +		WARN_ONCE(1, "Unexpected address width %u, defaulting to 3\n",
>> +			  nor->addr_width);
>> +		/* FALLTHROUGH */
>> +	case 3:
>> +		cmdaddr = addr & 0xFFFFFF;
>> +
>> +		cmdaddr |= (u32)cmd << 24;
>> +
>> +		temp = cpu_to_be32(cmdaddr);
>> +		aspeed_smc_to_fifo(chip->base, &temp, 4);
>> +		break;
>> +	case 4:
>> +		temp = cpu_to_be32(addr);
>> +		aspeed_smc_to_fifo(chip->base, &cmd, 1);
>> +		aspeed_smc_to_fifo(chip->base, &temp, 4);
>> +		break;
>> +	}
>> +}
>> +
>> +static ssize_t aspeed_smc_read_user(struct spi_nor *nor, loff_t from, size_t len,
>> +				    u_char *read_buf)
>> +{
>> +	struct aspeed_smc_chip *chip = nor->priv;
>> +
>> +	mutex_lock(&chip->controller->mutex);
>> +
>> +	aspeed_smc_start_user(nor);

I think this section of code lock+start_user should be in a prepare handler 
if I am correct ? and I could get rid of the lock in that case to rely on
the one of the spi nor.

>> +	aspeed_smc_send_cmd_addr(nor, nor->read_opcode, from);
> 
> I guess the dummy cycles are missing between address cycles and data cycles!
> Check nor->read_dummy ;)

ah yes, I meant to and I forgot to add it ... will do in v2. I suppose 
we are doing fine with the driver now because we only use normal speed.

> Or maybe this controller can really only perform READ operations but no FAST
> READ commands. Hence the "spi_nor_scan(&chip->nor, NULL, SPI_NOR_NORMAL)"
> below.

yes it can. I suppose the driver should also call spi_nor_set_flash_node() 
for module supporting it. that brings another question I would like 
to discuss for possible followups patches. 

The controller can be fine tuned for a particular chip module. The 
CE0 control register has a few bits to set the spi clock, the dummy 
cycles, the CS inactive width and there is also a register to setup 
read timings delay.
 
So you need to loop on these different settings to find a correct one
and for that, you use a gold image captured at low speed as reference.

But you need to know which chip module you are dealing with. would it 
be possible to expose the jedec to the controller for this purpose ? 
and then do the training to build the timing settings in the driver. 
This is a similar approach taken by some nand controllers.

Thanks,

C.

 
>> +	aspeed_smc_from_fifo(read_buf, chip->base, len);
>> +	aspeed_smc_stop_user(nor);
>> +
>> +	mutex_unlock(&chip->controller->mutex);
>> +
>> +	return len;
>> +}
> [...]
> 
> Best regards,
> 
> Cyrille
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] mtd/spi-nor: Add SPI memory controllers for Aspeed SoCs
  2016-10-18 16:39     ` Rob Herring
@ 2016-10-20  9:32       ` Cédric Le Goater
  -1 siblings, 0 replies; 12+ messages in thread
From: Cédric Le Goater @ 2016-10-20  9:32 UTC (permalink / raw)
  To: Rob Herring
  Cc: Milton Miller, devicetree, linux-mtd, Joel Stanley, Brian Norris,
	David Woodhouse

On 10/18/2016 06:39 PM, Rob Herring wrote:
> On Mon, Oct 17, 2016 at 06:57:07PM +0200, Cédric Le Goater wrote:
>> This driver adds mtd support for spi-nor attached to either or both of
>> the Firmware Memory Controller or the SPI Flash Controller (AST2400
>> only).
>>
>> The SMC controllers on the Aspeed AST2500 SoC are very similar to the
>> ones found on the AST2400. The differences are on the number of
>> supported flash modules and their default mappings in the SoC address
>> space.
>>
>> The Aspeed AST2500 has one SPI controller for the BMC firmware and two
>> for the host firmware. All controllers have now the same set of
>> registers compatible with the AST2400 FMC controller and the legacy
>> 'SMC' controller is fully gone.
>>
>> Based on previous work from Milton D. Miller II <miltonm@us.ibm.com>
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>  .../devicetree/bindings/mtd/aspeed-smc.txt         |  86 +++
>>  drivers/mtd/spi-nor/Kconfig                        |  12 +
>>  drivers/mtd/spi-nor/Makefile                       |   1 +
>>  drivers/mtd/spi-nor/aspeed-smc.c                   | 746 +++++++++++++++++++++
>>  4 files changed, 845 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/mtd/aspeed-smc.txt
>>  create mode 100644 drivers/mtd/spi-nor/aspeed-smc.c
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/aspeed-smc.txt b/Documentation/devicetree/bindings/mtd/aspeed-smc.txt
>> new file mode 100644
>> index 000000000000..f6bfa7761205
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mtd/aspeed-smc.txt
>> @@ -0,0 +1,86 @@
>> +* Aspeed Static Memory controller
>> +* Aspeed SPI Flash Controller
>> +
>> +The Static memory controller in the ast2400 supports 5 chip selects each
>> +can be attached to NAND, parallel NOR, or SPI NOR attached flash.  The
>> +Firmware Memory Controller in the ast2500 supports 3 chip selects, two of
>> +which are always in SPI-NOR mode and the third can be SPI-NOR or parallel
>> +flash.  The SPI flash controller in the ast2400 supports one of 2 chip
>> +selects selected by pinmux.  The two SPI flash controllers in the ast2500
>> +each support two chip selects.
>> +
>> +Required properties:
>> +  - compatible : Should be one of
>> +	"aspeed,ast2400-fmc" for the AST2400 Static Memory Controller
>> +	"aspeed,ast2400-smc" for the AST2400 SPI Flash Controller
>> +	"aspeed,ast2500-fmc" for the AST2500 Firmware SPI Memory Controller
>> +	"aspeed,ast2500-smc" for the AST2500 SPI Flash Controllers
>> +  - reg : the first contains the control register location and length,
>> +          the second contains the memory window mapping address and length
>> +  - clocks : The APB clock input to the controller
>> +  - #address-cells : must be 1 corresponding to chip select child binding
>> +  - #size-cells : must be 0 corresponding to chip select child binding
>> +
>> +Optional properties:
>> +  - aspeed,fmc-has-dma : controller supports DMA transfers
> 
> The compatible should imply this.

yes you are right. this is the case for the *fmc.

> 
>> +  - interrupts : Should contain the interrupt for the dma device if an fmc
>> +
>> +Child node required properties:
> 
> Need to say what the child nodes are.

yes again. the child node definitions really need more care. will do in v2.

Thanks for the review,

C. 


> 
>> +  - reg : must contain chip select number in first cell of address, must
>> +	  be 1 tuple long
>> +  - compatible : may contain "vendor,part", must include "jedec,spi-nor"
>> +		when attached to SPI flash (see spi-nor.txt binding).
>> +
>> +Child node optional properties:
>> +  - label           - (optional) name to assign to mtd, default os assigned
>> +
>> +Child node optional properties for SPI mode (may be ignored):
>> +  - spi-max-frequency - (optional) max frequency of spi bus
>> +  - spi-cpol        - (optional) Empty property indicating device requires
>> +			inverse clock polarity (CPOL) mode (boolean)
>> +  - spi-cpha        - (optional) Empty property indicating device requires
>> +			shifted clock phase (CPHA) mode (boolean)
>> +  - spi-tx-bus-width - (optional) The bus width(number of data wires) that
>> +                        used for MOSI. Defaults to 1 if not present.
>> +  - spi-rx-bus-width - (optional) The bus width(number of data wires) that
>> +                        used for MOSI. Defaults to 1 if not present.
>> +
>> +Grandchild node optional properties:
>> + - see mtd/partiton.txt for partitioning bindings and mtd naming
>> +
>> +
>> +Example:
>> +
>> +fmc: fmc@1e620000 {
>> +	compatible = "aspeed,ast2400-fmc";
>> +	reg = < 0x1e620000 0x94
>> +		0x20000000 0x02000000
>> +		0x22000000 0x02000000 >;
>> +	#address-cells = <1>;
>> +	#size-cells = <0>;
>> +	flash@0 {
>> +		reg = < 0 >;
>> +		compatible = "jedec,spi-nor" ;
>> +		label = "bmc";
> 
> label isn't really a defined property here. Belongs in jedec,spi-nor 
> binding if you want to add it.
> 
>> +		/* spi-max-frequency = <>; */
>> +		/* m25p,fast-read; */
>> +		#address-cells = <1>;
>> +		#size-cells = <1>;
>> +		boot@0 {
> 
> Use the newer style with all partitions under a 'partitions' node.
> 
>> +			label = "boot-loader";
>> +			reg = < 0 0x8000 >
> 
> Missing ';'
> 
>> +		}
>> +		image@8000 {
>> +			label = "kernel-image";
>> +			reg = < 0x8000 0x1f8000 >
> 
> ditto
> 
>> +		}
>> +	};
>> +	flash@1 {
>> +		reg = < 1 >;
>> +		compatible = "jedec,spi-nor" ;
>> +		label = "alt";
>> +		/* spi-max-frequency = <>; */
>> +		status = "fail";
>> +		/* m25p,fast-read; */
>> +	};
>> +};
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
> 


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] mtd/spi-nor: Add SPI memory controllers for Aspeed SoCs
@ 2016-10-20  9:32       ` Cédric Le Goater
  0 siblings, 0 replies; 12+ messages in thread
From: Cédric Le Goater @ 2016-10-20  9:32 UTC (permalink / raw)
  To: Rob Herring
  Cc: Milton Miller, devicetree, linux-mtd, Joel Stanley, Brian Norris,
	David Woodhouse

On 10/18/2016 06:39 PM, Rob Herring wrote:
> On Mon, Oct 17, 2016 at 06:57:07PM +0200, Cédric Le Goater wrote:
>> This driver adds mtd support for spi-nor attached to either or both of
>> the Firmware Memory Controller or the SPI Flash Controller (AST2400
>> only).
>>
>> The SMC controllers on the Aspeed AST2500 SoC are very similar to the
>> ones found on the AST2400. The differences are on the number of
>> supported flash modules and their default mappings in the SoC address
>> space.
>>
>> The Aspeed AST2500 has one SPI controller for the BMC firmware and two
>> for the host firmware. All controllers have now the same set of
>> registers compatible with the AST2400 FMC controller and the legacy
>> 'SMC' controller is fully gone.
>>
>> Based on previous work from Milton D. Miller II <miltonm@us.ibm.com>
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>  .../devicetree/bindings/mtd/aspeed-smc.txt         |  86 +++
>>  drivers/mtd/spi-nor/Kconfig                        |  12 +
>>  drivers/mtd/spi-nor/Makefile                       |   1 +
>>  drivers/mtd/spi-nor/aspeed-smc.c                   | 746 +++++++++++++++++++++
>>  4 files changed, 845 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/mtd/aspeed-smc.txt
>>  create mode 100644 drivers/mtd/spi-nor/aspeed-smc.c
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/aspeed-smc.txt b/Documentation/devicetree/bindings/mtd/aspeed-smc.txt
>> new file mode 100644
>> index 000000000000..f6bfa7761205
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mtd/aspeed-smc.txt
>> @@ -0,0 +1,86 @@
>> +* Aspeed Static Memory controller
>> +* Aspeed SPI Flash Controller
>> +
>> +The Static memory controller in the ast2400 supports 5 chip selects each
>> +can be attached to NAND, parallel NOR, or SPI NOR attached flash.  The
>> +Firmware Memory Controller in the ast2500 supports 3 chip selects, two of
>> +which are always in SPI-NOR mode and the third can be SPI-NOR or parallel
>> +flash.  The SPI flash controller in the ast2400 supports one of 2 chip
>> +selects selected by pinmux.  The two SPI flash controllers in the ast2500
>> +each support two chip selects.
>> +
>> +Required properties:
>> +  - compatible : Should be one of
>> +	"aspeed,ast2400-fmc" for the AST2400 Static Memory Controller
>> +	"aspeed,ast2400-smc" for the AST2400 SPI Flash Controller
>> +	"aspeed,ast2500-fmc" for the AST2500 Firmware SPI Memory Controller
>> +	"aspeed,ast2500-smc" for the AST2500 SPI Flash Controllers
>> +  - reg : the first contains the control register location and length,
>> +          the second contains the memory window mapping address and length
>> +  - clocks : The APB clock input to the controller
>> +  - #address-cells : must be 1 corresponding to chip select child binding
>> +  - #size-cells : must be 0 corresponding to chip select child binding
>> +
>> +Optional properties:
>> +  - aspeed,fmc-has-dma : controller supports DMA transfers
> 
> The compatible should imply this.

yes you are right. this is the case for the *fmc.

> 
>> +  - interrupts : Should contain the interrupt for the dma device if an fmc
>> +
>> +Child node required properties:
> 
> Need to say what the child nodes are.

yes again. the child node definitions really need more care. will do in v2.

Thanks for the review,

C. 


> 
>> +  - reg : must contain chip select number in first cell of address, must
>> +	  be 1 tuple long
>> +  - compatible : may contain "vendor,part", must include "jedec,spi-nor"
>> +		when attached to SPI flash (see spi-nor.txt binding).
>> +
>> +Child node optional properties:
>> +  - label           - (optional) name to assign to mtd, default os assigned
>> +
>> +Child node optional properties for SPI mode (may be ignored):
>> +  - spi-max-frequency - (optional) max frequency of spi bus
>> +  - spi-cpol        - (optional) Empty property indicating device requires
>> +			inverse clock polarity (CPOL) mode (boolean)
>> +  - spi-cpha        - (optional) Empty property indicating device requires
>> +			shifted clock phase (CPHA) mode (boolean)
>> +  - spi-tx-bus-width - (optional) The bus width(number of data wires) that
>> +                        used for MOSI. Defaults to 1 if not present.
>> +  - spi-rx-bus-width - (optional) The bus width(number of data wires) that
>> +                        used for MOSI. Defaults to 1 if not present.
>> +
>> +Grandchild node optional properties:
>> + - see mtd/partiton.txt for partitioning bindings and mtd naming
>> +
>> +
>> +Example:
>> +
>> +fmc: fmc@1e620000 {
>> +	compatible = "aspeed,ast2400-fmc";
>> +	reg = < 0x1e620000 0x94
>> +		0x20000000 0x02000000
>> +		0x22000000 0x02000000 >;
>> +	#address-cells = <1>;
>> +	#size-cells = <0>;
>> +	flash@0 {
>> +		reg = < 0 >;
>> +		compatible = "jedec,spi-nor" ;
>> +		label = "bmc";
> 
> label isn't really a defined property here. Belongs in jedec,spi-nor 
> binding if you want to add it.
> 
>> +		/* spi-max-frequency = <>; */
>> +		/* m25p,fast-read; */
>> +		#address-cells = <1>;
>> +		#size-cells = <1>;
>> +		boot@0 {
> 
> Use the newer style with all partitions under a 'partitions' node.
> 
>> +			label = "boot-loader";
>> +			reg = < 0 0x8000 >
> 
> Missing ';'
> 
>> +		}
>> +		image@8000 {
>> +			label = "kernel-image";
>> +			reg = < 0x8000 0x1f8000 >
> 
> ditto
> 
>> +		}
>> +	};
>> +	flash@1 {
>> +		reg = < 1 >;
>> +		compatible = "jedec,spi-nor" ;
>> +		label = "alt";
>> +		/* spi-max-frequency = <>; */
>> +		status = "fail";
>> +		/* m25p,fast-read; */
>> +	};
>> +};
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] mtd/spi-nor: Add SPI memory controllers for Aspeed SoCs
  2016-10-20  9:17   ` Cédric Le Goater
@ 2016-10-20 14:33     ` Cyrille Pitchen
  2016-10-21 11:25       ` Cédric Le Goater
  0 siblings, 1 reply; 12+ messages in thread
From: Cyrille Pitchen @ 2016-10-20 14:33 UTC (permalink / raw)
  To: Cédric Le Goater, linux-mtd

Hi Cédric,

Le 20/10/2016 à 11:17, Cédric Le Goater a écrit :
> Hello Cyrille,
> 
> Thanks for taking a look, some answers and more questions below.
> 
> On 10/19/2016 07:06 PM, Cyrille Pitchen wrote:
>> Hi Cédric,
>>
>> Le 17/10/2016 à 18:57, Cédric Le Goater a écrit :
>>> This driver adds mtd support for spi-nor attached to either or both of
>>> the Firmware Memory Controller or the SPI Flash Controller (AST2400
>>> only).
>>>
>>> The SMC controllers on the Aspeed AST2500 SoC are very similar to the
>>> ones found on the AST2400. The differences are on the number of
>>> supported flash modules and their default mappings in the SoC address
>>> space.
>>>
>>> The Aspeed AST2500 has one SPI controller for the BMC firmware and two
>>> for the host firmware. All controllers have now the same set of
>>> registers compatible with the AST2400 FMC controller and the legacy
>>> 'SMC' controller is fully gone.
>>>
>>> Based on previous work from Milton D. Miller II <miltonm@us.ibm.com>
>>>
>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>> ---
>> [...]
>>> +
>>> +static void aspeed_smc_send_cmd_addr(struct spi_nor *nor, u8 cmd, u32 addr)
>>> +{
>>> +	struct aspeed_smc_chip *chip = nor->priv;
>>> +	__be32 temp;
>>> +	u32 cmdaddr;
>>> +
>>> +	switch (nor->addr_width) {
>>> +	default:
>>> +		WARN_ONCE(1, "Unexpected address width %u, defaulting to 3\n",
>>> +			  nor->addr_width);
>>> +		/* FALLTHROUGH */
>>> +	case 3:
>>> +		cmdaddr = addr & 0xFFFFFF;
>>> +
>>> +		cmdaddr |= (u32)cmd << 24;
>>> +
>>> +		temp = cpu_to_be32(cmdaddr);
>>> +		aspeed_smc_to_fifo(chip->base, &temp, 4);
>>> +		break;
>>> +	case 4:
>>> +		temp = cpu_to_be32(addr);
>>> +		aspeed_smc_to_fifo(chip->base, &cmd, 1);
>>> +		aspeed_smc_to_fifo(chip->base, &temp, 4);
>>> +		break;
>>> +	}
>>> +}
>>> +
>>> +static ssize_t aspeed_smc_read_user(struct spi_nor *nor, loff_t from, size_t len,
>>> +				    u_char *read_buf)
>>> +{
>>> +	struct aspeed_smc_chip *chip = nor->priv;
>>> +
>>> +	mutex_lock(&chip->controller->mutex);
>>> +
>>> +	aspeed_smc_start_user(nor);
> 
> I think this section of code lock+start_user should be in a prepare handler 
> if I am correct ? and I could get rid of the lock in that case to rely on
> the one of the spi nor.
> 

For the mutex, looking at your driver, you lock it at the beginning then unlock
at the end of the four handlers:
- nor->read()
- nor->write()
- nor->read_reg()
- nor->write_reg()

If this is the only purpose of this mutex, yes, you can remove it and relie on
the internal spi-nor one (nor->lock), which is locked from
spi_nor_lock_and_prep() hence at the beginning of spi_nor_erase(),
spi_nor_read() and spi_nor_write() calls.

However, about moving the call of aspeed_smc_start_user() into a new prepare
handler, I don't know whether it would actually fit your needs: for sure, 
aspeed_smc_start_user() would be no longer called at some points.

2 examples:

A - spi_nor_write() calls:
1 - spi_nor_lock_and_prep() -> nor->prepare()
2 - write_enable() -> nor->write_reg()
3 - nor->write()
4 - spi_nor_wait_till_ready() -> nor->read_reg()
5 - spi_nor_unlock_and_unprep() -> nor->unprepare()

With your current driver, aspeed_smc_start_user()/aspeed_smc_stop_user() are
called 3 times, indeed each time a command is sent on the SPI bus:
WRITE ENABLE, PAGE PROGRAM, READ STATUS

Then if you move then into spi_nor_[un]lock_and_[un]prep(), those two functions
would only be called once each.


B - spi_nor_scan() calls:
1 - spi_nor_read_id() -> nor->read_reg()

but spi_nor_lock_and_prep() is not called before, so if you nead to call
aspeed_smc_start_user() before sending any command on the SPI bus, it won't
work.

I don't know how your SPI controller works but maybe using the nor->prepare()
handler is not what you want!


>>> +	aspeed_smc_send_cmd_addr(nor, nor->read_opcode, from);
>>
>> I guess the dummy cycles are missing between address cycles and data cycles!
>> Check nor->read_dummy ;)
> 
> ah yes, I meant to and I forgot to add it ... will do in v2. I suppose 
> we are doing fine with the driver now because we only use normal speed.
> 

Indeed, the legacy READ (03h) command has no dummy cycles as opposed to all
other FAST READ commands. Of course, the READ command is far much slower.

>> Or maybe this controller can really only perform READ operations but no FAST
>> READ commands. Hence the "spi_nor_scan(&chip->nor, NULL, SPI_NOR_NORMAL)"
>> below.
> 
> yes it can. I suppose the driver should also call spi_nor_set_flash_node() 
> for module supporting it. that brings another question I would like 
> to discuss for possible followups patches. 
> 

Calling spi_nor_set_flash_node() with the relevant parameter tells
spi_nor_scan() which DT node to parse to tune some settings.
For instance, you could add the "m25p,fast-read" DT property so spi_nor_scan()
would select Fast Read operations rather than Read operations.

> The controller can be fine tuned for a particular chip module. The 
> CE0 control register has a few bits to set the spi clock, the dummy 
> cycles, the CS inactive width and there is also a register to setup 
> read timings delay.
>  
> So you need to loop on these different settings to find a correct one
> and for that, you use a gold image captured at low speed as reference.
> 
> But you need to know which chip module you are dealing with. would it 
> be possible to expose the jedec to the controller for this purpose ? 
> and then do the training to build the timing settings in the driver. 
> This is a similar approach taken by some nand controllers.
> 

I'm not sure to fully understand your needs.
The spi clock is often provided in the device tree, on the controller or memory
node, with the "spi-max-frequency" DT property.
The number of dummy cycles is set in nor->read_dummy by spi_nor_scan()
according to the actual memory model and the selected (Fast) Read op code.
For chip-select timings, those settings aren't currently handled by
spi_nor_scan() but might be supported inside the SPI controller driver through
DT properties if needed.

I don't know why your SPI controller driver needs to know the exact memory
model to work. Aren't the pieces of information provided by spi-nor.c enough?
Actually, for this point I don't quite exactly understand your needs! :)

Best regards,

Cyrille


> Thanks,
> 
> C.
> 
>  
>>> +	aspeed_smc_from_fifo(read_buf, chip->base, len);
>>> +	aspeed_smc_stop_user(nor);
>>> +
>>> +	mutex_unlock(&chip->controller->mutex);
>>> +
>>> +	return len;
>>> +}
>> [...]
>>
>> Best regards,
>>
>> Cyrille
>>
>> ______________________________________________________
>> Linux MTD discussion mailing list
>> http://lists.infradead.org/mailman/listinfo/linux-mtd/
>>
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] mtd/spi-nor: Add SPI memory controllers for Aspeed SoCs
  2016-10-20 14:33     ` Cyrille Pitchen
@ 2016-10-21 11:25       ` Cédric Le Goater
  0 siblings, 0 replies; 12+ messages in thread
From: Cédric Le Goater @ 2016-10-21 11:25 UTC (permalink / raw)
  To: Cyrille Pitchen, linux-mtd

Hello Cyrille,

On 10/20/2016 04:33 PM, Cyrille Pitchen wrote:
> Hi Cédric,
> 
> Le 20/10/2016 à 11:17, Cédric Le Goater a écrit :
>> Hello Cyrille,
>>
>> Thanks for taking a look, some answers and more questions below.
>>
>> On 10/19/2016 07:06 PM, Cyrille Pitchen wrote:
>>> Hi Cédric,
>>>
>>> Le 17/10/2016 à 18:57, Cédric Le Goater a écrit :
>>>> This driver adds mtd support for spi-nor attached to either or both of
>>>> the Firmware Memory Controller or the SPI Flash Controller (AST2400
>>>> only).
>>>>
>>>> The SMC controllers on the Aspeed AST2500 SoC are very similar to the
>>>> ones found on the AST2400. The differences are on the number of
>>>> supported flash modules and their default mappings in the SoC address
>>>> space.
>>>>
>>>> The Aspeed AST2500 has one SPI controller for the BMC firmware and two
>>>> for the host firmware. All controllers have now the same set of
>>>> registers compatible with the AST2400 FMC controller and the legacy
>>>> 'SMC' controller is fully gone.
>>>>
>>>> Based on previous work from Milton D. Miller II <miltonm@us.ibm.com>
>>>>
>>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>>> ---
>>> [...]
>>>> +
>>>> +static void aspeed_smc_send_cmd_addr(struct spi_nor *nor, u8 cmd, u32 addr)
>>>> +{
>>>> +	struct aspeed_smc_chip *chip = nor->priv;
>>>> +	__be32 temp;
>>>> +	u32 cmdaddr;
>>>> +
>>>> +	switch (nor->addr_width) {
>>>> +	default:
>>>> +		WARN_ONCE(1, "Unexpected address width %u, defaulting to 3\n",
>>>> +			  nor->addr_width);
>>>> +		/* FALLTHROUGH */
>>>> +	case 3:
>>>> +		cmdaddr = addr & 0xFFFFFF;
>>>> +
>>>> +		cmdaddr |= (u32)cmd << 24;
>>>> +
>>>> +		temp = cpu_to_be32(cmdaddr);
>>>> +		aspeed_smc_to_fifo(chip->base, &temp, 4);
>>>> +		break;
>>>> +	case 4:
>>>> +		temp = cpu_to_be32(addr);
>>>> +		aspeed_smc_to_fifo(chip->base, &cmd, 1);
>>>> +		aspeed_smc_to_fifo(chip->base, &temp, 4);
>>>> +		break;
>>>> +	}
>>>> +}
>>>> +
>>>> +static ssize_t aspeed_smc_read_user(struct spi_nor *nor, loff_t from, size_t len,
>>>> +				    u_char *read_buf)
>>>> +{
>>>> +	struct aspeed_smc_chip *chip = nor->priv;
>>>> +
>>>> +	mutex_lock(&chip->controller->mutex);
>>>> +
>>>> +	aspeed_smc_start_user(nor);
>>
>> I think this section of code lock+start_user should be in a prepare handler 
>> if I am correct ? and I could get rid of the lock in that case to rely on
>> the one of the spi nor.
>>
> 
> For the mutex, looking at your driver, you lock it at the beginning then unlock
> at the end of the four handlers:
> - nor->read()
> - nor->write()
> - nor->read_reg()
> - nor->write_reg()
> 
> If this is the only purpose of this mutex, yes, you can remove it and relie on
> the internal spi-nor one (nor->lock), which is locked from
> spi_nor_lock_and_prep() hence at the beginning of spi_nor_erase(),
> spi_nor_read() and spi_nor_write() calls.
> 
> However, about moving the call of aspeed_smc_start_user() into a new prepare
> handler, I don't know whether it would actually fit your needs: for sure, 
> aspeed_smc_start_user() would be no longer called at some points.
> 
> 2 examples:
> 
> A - spi_nor_write() calls:
> 1 - spi_nor_lock_and_prep() -> nor->prepare()
> 2 - write_enable() -> nor->write_reg()
> 3 - nor->write()
> 4 - spi_nor_wait_till_ready() -> nor->read_reg()
> 5 - spi_nor_unlock_and_unprep() -> nor->unprepare()
> 
> With your current driver, aspeed_smc_start_user()/aspeed_smc_stop_user() are
> called 3 times, indeed each time a command is sent on the SPI bus:
> WRITE ENABLE, PAGE PROGRAM, READ STATUS
> 
> Then if you move then into spi_nor_[un]lock_and_[un]prep(), those two functions
> would only be called once each.
> 
> 
> B - spi_nor_scan() calls:
> 1 - spi_nor_read_id() -> nor->read_reg()
> 
> but spi_nor_lock_and_prep() is not called before, so if you nead to call
> aspeed_smc_start_user() before sending any command on the SPI bus, it won't
> work.

ok. So I need to keep it for now as the driver only supports the 
'User mode' of the controller.

In 'User mode', the controller sends commands to the module doing 
writes at the base address where the module is mapped and gets 
the results doing reads. So whatever is done, we need to make sure
this mode is enforced.

There is another mode called 'Command Mode' in the specs (the naming
is rather confusing) in which you can access the contents of the 
flash doing memory accesses in the memory window in which the module 
is mapped. The controller uses the setting in the CE control register
to generate the commands. This is not supported yet.

> I don't know how your SPI controller works but maybe using the nor->prepare()
> handler is not what you want!

Thanks a lot for the analysis.

>>>> +	aspeed_smc_send_cmd_addr(nor, nor->read_opcode, from);
>>>
>>> I guess the dummy cycles are missing between address cycles and data cycles!
>>> Check nor->read_dummy ;)
>>
>> ah yes, I meant to and I forgot to add it ... will do in v2. I suppose 
>> we are doing fine with the driver now because we only use normal speed.
>>
> 
> Indeed, the legacy READ (03h) command has no dummy cycles as opposed to all
> other FAST READ commands. Of course, the READ command is far much slower.
> 
>>> Or maybe this controller can really only perform READ operations but no FAST
>>> READ commands. Hence the "spi_nor_scan(&chip->nor, NULL, SPI_NOR_NORMAL)"
>>> below.
>>
>> yes it can. I suppose the driver should also call spi_nor_set_flash_node() 
>> for module supporting it. that brings another question I would like 
>> to discuss for possible followups patches. 
>>
> 
> Calling spi_nor_set_flash_node() with the relevant parameter tells
> spi_nor_scan() which DT node to parse to tune some settings.
> For instance, you could add the "m25p,fast-read" DT property so spi_nor_scan()
> would select Fast Read operations rather than Read operations.

Yes. That is something I will try pretty soon on the boards we have.

>> The controller can be fine tuned for a particular chip module. The 
>> CE0 control register has a few bits to set the spi clock, the dummy 
>> cycles, the CS inactive width and there is also a register to setup 
>> read timings delay.
>>  
>> So you need to loop on these different settings to find a correct one
>> and for that, you use a gold image captured at low speed as reference.
>>
>> But you need to know which chip module you are dealing with. would it 
>> be possible to expose the jedec to the controller for this purpose ? 
>> and then do the training to build the timing settings in the driver. 
>> This is a similar approach taken by some nand controllers.
>>
> 
> I'm not sure to fully understand your needs.
> The spi clock is often provided in the device tree, on the controller or memory
> node, with the "spi-max-frequency" DT property.

yes but it is possible also to discover what is the best timing at runtime 
depending on the chip model found and not rely on the DT for that. That is 
what I am trying to explain poorly. 

> The number of dummy cycles is set in nor->read_dummy by spi_nor_scan()
> according to the actual memory model and the selected (Fast) Read op code.
> For chip-select timings, those settings aren't currently handled by
> spi_nor_scan() but might be supported inside the SPI controller driver 
> through DT properties if needed.
>
> I don't know why your SPI controller driver needs to know the exact memory
> model to work. Aren't the pieces of information provided by spi-nor.c enough?
> Actually, for this point I don't quite exactly understand your needs! :)

he. Let's start with a non optimized version, then I will check the fast 
read op with the different boards/modules we have. Timings will come later, 
DMA also.

Thanks for the review.

C. 


> 
> Best regards,
> 
> Cyrille
> 
> 
>> Thanks,
>>
>> C.
>>
>>  
>>>> +	aspeed_smc_from_fifo(read_buf, chip->base, len);
>>>> +	aspeed_smc_stop_user(nor);
>>>> +
>>>> +	mutex_unlock(&chip->controller->mutex);
>>>> +
>>>> +	return len;
>>>> +}
>>> [...]
>>>
>>> Best regards,
>>>
>>> Cyrille
>>>
>>> ______________________________________________________
>>> Linux MTD discussion mailing list
>>> http://lists.infradead.org/mailman/listinfo/linux-mtd/
>>>
>>
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] mtd/spi-nor: Add SPI memory controllers for Aspeed SoCs
  2016-10-18 16:39     ` Rob Herring
@ 2016-11-19  6:36       ` Cédric Le Goater
  -1 siblings, 0 replies; 12+ messages in thread
From: Cédric Le Goater @ 2016-11-19  6:36 UTC (permalink / raw)
  To: Rob Herring
  Cc: Milton Miller, devicetree, linux-mtd, Joel Stanley, Brian Norris,
	David Woodhouse

Hello Rob,

>> +Example:
>> +
>> +fmc: fmc@1e620000 {
>> +	compatible = "aspeed,ast2400-fmc";
>> +	reg = < 0x1e620000 0x94
>> +		0x20000000 0x02000000
>> +		0x22000000 0x02000000 >;
>> +	#address-cells = <1>;
>> +	#size-cells = <0>;
>> +	flash@0 {
>> +		reg = < 0 >;
>> +		compatible = "jedec,spi-nor" ;
>> +		label = "bmc";
> 
> label isn't really a defined property here. Belongs in jedec,spi-nor 
> binding if you want to add it.
> 

I am not sure I understand the suggestion.

Do you mean that we should rename the device node to something 
like "bmc@0" to identify a flash module on the system ?

This is a bit problematic for user space today as we need to sort 
out which flash modules are for the BMC or for the host, each having 
a primary and alternate. So the "label" was a practical way for 
identification.

Thanks,

C.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] mtd/spi-nor: Add SPI memory controllers for Aspeed SoCs
@ 2016-11-19  6:36       ` Cédric Le Goater
  0 siblings, 0 replies; 12+ messages in thread
From: Cédric Le Goater @ 2016-11-19  6:36 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-mtd, David Woodhouse, Brian Norris, devicetree,
	Joel Stanley, Milton Miller

Hello Rob,

>> +Example:
>> +
>> +fmc: fmc@1e620000 {
>> +	compatible = "aspeed,ast2400-fmc";
>> +	reg = < 0x1e620000 0x94
>> +		0x20000000 0x02000000
>> +		0x22000000 0x02000000 >;
>> +	#address-cells = <1>;
>> +	#size-cells = <0>;
>> +	flash@0 {
>> +		reg = < 0 >;
>> +		compatible = "jedec,spi-nor" ;
>> +		label = "bmc";
> 
> label isn't really a defined property here. Belongs in jedec,spi-nor 
> binding if you want to add it.
> 

I am not sure I understand the suggestion.

Do you mean that we should rename the device node to something 
like "bmc@0" to identify a flash module on the system ?

This is a bit problematic for user space today as we need to sort 
out which flash modules are for the BMC or for the host, each having 
a primary and alternate. So the "label" was a practical way for 
identification.

Thanks,

C.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2016-11-19  6:37 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-17 16:57 [PATCH] mtd/spi-nor: Add SPI memory controllers for Aspeed SoCs Cédric Le Goater
2016-10-17 16:57 ` Cédric Le Goater
     [not found] ` <1476723427-13243-1-git-send-email-clg-Bxea+6Xhats@public.gmane.org>
2016-10-18 16:39   ` Rob Herring
2016-10-18 16:39     ` Rob Herring
2016-10-20  9:32     ` Cédric Le Goater
2016-10-20  9:32       ` Cédric Le Goater
2016-11-19  6:36     ` Cédric Le Goater
2016-11-19  6:36       ` Cédric Le Goater
2016-10-19 17:06 ` Cyrille Pitchen
2016-10-20  9:17   ` Cédric Le Goater
2016-10-20 14:33     ` Cyrille Pitchen
2016-10-21 11:25       ` Cédric Le Goater

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.