Linux-mtd Archive on lore.kernel.org
 help / Atom feed
* [PATCH v3 0/4] Add Macronix MX25F0A MFD driver for raw nand and spi
@ 2019-04-15  9:23 Mason Yang
  2019-04-15  9:23 ` [PATCH v3 1/4] mfd: Add Macronix MX25F0A MFD controller driver Mason Yang
                   ` (3 more replies)
  0 siblings, 4 replies; 21+ messages in thread
From: Mason Yang @ 2019-04-15  9:23 UTC (permalink / raw)
  To: broonie, marek.vasut, linux-kernel, linux-spi, bbrezillon, dwmw2,
	lee.jones, robh+dt, mark.rutland, computersforpeace, paul.burton,
	stefan, christophe.kerello, liang.yang, geert, devicetree,
	marcel.ziswiler, linux-mtd, richard, miquel.raynal
  Cc: juliensu, Mason Yang, zhengxunli

Hi,

v3 patch is to rename the title of SPI controller driver.
"Patch Macronix SPI controller driver according to MX25F0A MFD driver"

v2s patches is to support Macronix MX25F0A MFD driver 
for raw nand and spi controller which is separated 
form previous patchset:

https://patchwork.kernel.org/patch/10874679/

thanks for your review.

best regards,
Mason

Mason Yang (4):
  mfd: Add Macronix MX25F0A MFD controller driver
  mtd: rawnand: Add Macronix MX25F0A NAND controller
  spi: Patch Macronix SPI controller driver according to MX25F0A MFD
    driver
  dt-bindings: mfd: Document Macronix MX25F0A controller bindings

 .../devicetree/bindings/mfd/mxic-mx25f0a.txt       |  51 ++++
 drivers/mfd/Kconfig                                |   9 +
 drivers/mfd/Makefile                               |   1 +
 drivers/mfd/mxic-mx25f0a.c                         |  84 ++++++
 drivers/mtd/nand/raw/Kconfig                       |   6 +
 drivers/mtd/nand/raw/Makefile                      |   1 +
 drivers/mtd/nand/raw/mxic_nand.c                   | 294 +++++++++++++++++++++
 drivers/spi/spi-mxic.c                             | 275 ++++---------------
 include/linux/mfd/mxic-mx25f0a.h                   | 175 ++++++++++++
 9 files changed, 670 insertions(+), 226 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/mxic-mx25f0a.txt
 create mode 100644 drivers/mfd/mxic-mx25f0a.c
 create mode 100644 drivers/mtd/nand/raw/mxic_nand.c
 create mode 100644 include/linux/mfd/mxic-mx25f0a.h

-- 
1.9.1


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

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

* [PATCH v3 1/4]  mfd: Add Macronix MX25F0A MFD controller driver
  2019-04-15  9:23 [PATCH v3 0/4] Add Macronix MX25F0A MFD driver for raw nand and spi Mason Yang
@ 2019-04-15  9:23 ` Mason Yang
  2019-05-12 13:08   ` Miquel Raynal
  2019-04-15  9:23 ` [PATCH v3 2/4] mtd: rawnand: Add Macronix MX25F0A NAND controller Mason Yang
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 21+ messages in thread
From: Mason Yang @ 2019-04-15  9:23 UTC (permalink / raw)
  To: broonie, marek.vasut, linux-kernel, linux-spi, bbrezillon, dwmw2,
	lee.jones, robh+dt, mark.rutland, computersforpeace, paul.burton,
	stefan, christophe.kerello, liang.yang, geert, devicetree,
	marcel.ziswiler, linux-mtd, richard, miquel.raynal
  Cc: juliensu, Mason Yang, zhengxunli

Add a driver for Macronix MX25F0A multifunction device controller.

Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
---
 drivers/mfd/Kconfig              |   9 ++
 drivers/mfd/Makefile             |   1 +
 drivers/mfd/mxic-mx25f0a.c       |  84 +++++++++++++++++++
 include/linux/mfd/mxic-mx25f0a.h | 175 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 269 insertions(+)
 create mode 100644 drivers/mfd/mxic-mx25f0a.c
 create mode 100644 include/linux/mfd/mxic-mx25f0a.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 26ad646..7e99e93 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -823,6 +823,15 @@ config MFD_MAX8998
 	  additional drivers must be enabled in order to use the functionality
 	  of the device.
 
+config MFD_MXIC_MX25F0A
+	tristate "Macronix mx25f0a multifunction device support"
+	select MFD_CORE
+	help
+	  This supports for Macronix mx25f0a multifunction device controller
+	  for raw nand or spi. You have to select individual components like
+	  raw nand controller or spi host controller under the corresponding
+	  menus.
+
 config MFD_MT6397
 	tristate "MediaTek MT6397 PMIC Support"
 	select MFD_CORE
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index b4569ed7..dcfe8fd 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -163,6 +163,7 @@ max8925-objs			:= max8925-core.o max8925-i2c.o
 obj-$(CONFIG_MFD_MAX8925)	+= max8925.o
 obj-$(CONFIG_MFD_MAX8997)	+= max8997.o max8997-irq.o
 obj-$(CONFIG_MFD_MAX8998)	+= max8998.o max8998-irq.o
+obj-$(CONFIG_MFD_MXIC_MX25F0A)	+= mxic-mx25f0a.o
 
 pcf50633-objs			:= pcf50633-core.o pcf50633-irq.o
 obj-$(CONFIG_MFD_PCF50633)	+= pcf50633.o
diff --git a/drivers/mfd/mxic-mx25f0a.c b/drivers/mfd/mxic-mx25f0a.c
new file mode 100644
index 0000000..a884d97
--- /dev/null
+++ b/drivers/mfd/mxic-mx25f0a.c
@@ -0,0 +1,84 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// Copyright (C) 2019 Macronix International Co., Ltd.
+//
+// Author:
+//	Mason Yang <masonccyang@mxic.com.tw>
+//
+
+#include <linux/clk.h>
+#include <linux/mfd/mxic-mx25f0a.h>
+#include <linux/mfd/core.h>
+
+static const struct mfd_cell mx25f0a_nand_ctlr = {
+	.name = "mxic-nand-ctlr",
+	.of_compatible = "mxicy,mx25f0a-nand-ctlr",
+};
+
+static const struct mfd_cell mx25f0a_spi_ctlr = {
+	.name = "mxic-spi",
+	.of_compatible = "mxicy,mx25f0a-spi",
+};
+
+static int mx25f0a_mfd_probe(struct platform_device *pdev)
+{
+	const struct mfd_cell *cell;
+	struct mx25f0a_mfd *mxic;
+	struct resource *res;
+	int ret;
+
+	ret = of_device_is_compatible(pdev->dev.of_node->child,
+				      "jedec,spi-nor");
+	if (ret)
+		cell = &mx25f0a_spi_ctlr;
+	else
+		cell = &mx25f0a_nand_ctlr;
+
+	mxic = devm_kzalloc(&pdev->dev, sizeof(*mxic), GFP_KERNEL);
+	if (!mxic)
+		return -ENOMEM;
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
+	mxic->regs = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(mxic->regs))
+		return PTR_ERR(mxic->regs);
+
+	if (cell == &mx25f0a_spi_ctlr) {
+		mxic->ps_clk = devm_clk_get(&pdev->dev, "ps_clk");
+		if (IS_ERR(mxic->ps_clk))
+			return PTR_ERR(mxic->ps_clk);
+
+		mxic->send_clk = devm_clk_get(&pdev->dev, "send_clk");
+		if (IS_ERR(mxic->send_clk))
+			return PTR_ERR(mxic->send_clk);
+
+		mxic->send_dly_clk = devm_clk_get(&pdev->dev, "send_dly_clk");
+		if (IS_ERR(mxic->send_dly_clk))
+			return PTR_ERR(mxic->send_dly_clk);
+	}
+
+	platform_set_drvdata(pdev, mxic);
+
+	ret = devm_mfd_add_devices(&pdev->dev, -1, cell, 1, NULL, 0, NULL);
+
+	return ret;
+}
+
+static const struct of_device_id mx25f0a_mfd_of_match[] = {
+	{ .compatible = "mxic,mx25f0a", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, mx25f0a_mfd_of_match);
+
+static struct platform_driver mx25f0a_mfd_driver = {
+	.probe		= mx25f0a_mfd_probe,
+	.driver = {
+		.name =	"mx25f0a-mfd",
+		.of_match_table = mx25f0a_mfd_of_match,
+	},
+};
+module_platform_driver(mx25f0a_mfd_driver);
+
+MODULE_AUTHOR("Mason Yang <masonccyang@mxic.com.tw>");
+MODULE_DESCRIPTION("MX25F0A controller MFD driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mfd/mxic-mx25f0a.h b/include/linux/mfd/mxic-mx25f0a.h
new file mode 100644
index 0000000..cb7bf19
--- /dev/null
+++ b/include/linux/mfd/mxic-mx25f0a.h
@@ -0,0 +1,175 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+//
+// Copyright (C) 2019 Macronix International Co., Ltd.
+//
+// Author:
+//	Mason Yang <masonccyang@mxic.com.tw>
+//
+
+#ifndef __MFD_MXIC_MX25F0A_H
+#define __MFD_MXIC_MX25F0A_H
+
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <linux/mtd/mtd.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+#define HC_CFG			0x0
+#define HC_CFG_IF_CFG(x)	((x) << 27)
+#define HC_CFG_DUAL_SLAVE	BIT(31)
+#define HC_CFG_INDIVIDUAL	BIT(30)
+#define HC_CFG_NIO(x)		(((x) / 4) << 27)
+#define HC_CFG_TYPE(s, t)	((t) << (23 + ((s) * 2)))
+#define HC_CFG_TYPE_SPI_NOR	0
+#define HC_CFG_TYPE_SPI_NAND	1
+#define HC_CFG_TYPE_SPI_RAM	2
+#define HC_CFG_TYPE_RAW_NAND	3
+#define HC_CFG_SLV_ACT(x)	((x) << 21)
+#define HC_CFG_CLK_PH_EN	BIT(20)
+#define HC_CFG_CLK_POL_INV	BIT(19)
+#define HC_CFG_BIG_ENDIAN	BIT(18)
+#define HC_CFG_DATA_PASS	BIT(17)
+#define HC_CFG_IDLE_SIO_LVL(x)	((x) << 16)
+#define HC_CFG_MAN_START_EN	BIT(3)
+#define HC_CFG_MAN_START	BIT(2)
+#define HC_CFG_MAN_CS_EN	BIT(1)
+#define HC_CFG_MAN_CS_ASSERT	BIT(0)
+
+#define INT_STS			0x4
+#define INT_STS_EN		0x8
+#define INT_SIG_EN		0xc
+#define INT_STS_ALL		GENMASK(31, 0)
+#define INT_RDY_PIN		BIT(26)
+#define INT_RDY_SR		BIT(25)
+#define INT_LNR_SUSP		BIT(24)
+#define INT_ECC_ERR		BIT(17)
+#define INT_CRC_ERR		BIT(16)
+#define INT_LWR_DIS		BIT(12)
+#define INT_LRD_DIS		BIT(11)
+#define INT_SDMA_INT		BIT(10)
+#define INT_DMA_FINISH		BIT(9)
+#define INT_RX_NOT_FULL		BIT(3)
+#define INT_RX_NOT_EMPTY	BIT(2)
+#define INT_TX_NOT_FULL		BIT(1)
+#define INT_TX_EMPTY		BIT(0)
+
+#define HC_EN			0x10
+#define HC_EN_BIT		BIT(0)
+
+#define TXD(x)			(0x14 + ((x) * 4))
+#define RXD			0x24
+
+#define SS_CTRL(s)		(0x30 + ((s) * 4))
+#define LRD_CFG			0x44
+#define LWR_CFG			0x80
+#define RWW_CFG			0x70
+#define OP_READ			BIT(23)
+#define OP_DUMMY_CYC(x)		((x) << 17)
+#define OP_ADDR_BYTES(x)	((x) << 14)
+#define OP_CMD_BYTES(x)		(((x) - 1) << 13)
+#define OP_OCTA_CRC_EN		BIT(12)
+#define OP_DQS_EN		BIT(11)
+#define OP_ENHC_EN		BIT(10)
+#define OP_PREAMBLE_EN		BIT(9)
+#define OP_DATA_DDR		BIT(8)
+#define OP_DATA_BUSW(x)		((x) << 6)
+#define OP_ADDR_DDR		BIT(5)
+#define OP_ADDR_BUSW(x)		((x) << 3)
+#define OP_CMD_DDR		BIT(2)
+#define OP_CMD_BUSW(x)		(x)
+#define OP_BUSW_1		0
+#define OP_BUSW_2		1
+#define OP_BUSW_4		2
+#define OP_BUSW_8		3
+
+#define OCTA_CRC		0x38
+#define OCTA_CRC_IN_EN(s)	BIT(3 + ((s) * 16))
+#define OCTA_CRC_CHUNK(s, x)	((fls((x) / 32)) << (1 + ((s) * 16)))
+#define OCTA_CRC_OUT_EN(s)	BIT(0 + ((s) * 16))
+
+#define ONFI_DIN_CNT(s)		(0x3c + (s))
+
+#define LRD_CTRL		0x48
+#define RWW_CTRL		0x74
+#define LWR_CTRL		0x84
+#define LMODE_EN		BIT(31)
+#define LMODE_SLV_ACT(x)	((x) << 21)
+#define LMODE_CMD1(x)		((x) << 8)
+#define LMODE_CMD0(x)		(x)
+
+#define LRD_ADDR		0x4c
+#define LWR_ADDR		0x88
+#define LRD_RANGE		0x50
+#define LWR_RANGE		0x8c
+
+#define AXI_SLV_ADDR		0x54
+
+#define DMAC_RD_CFG		0x58
+#define DMAC_WR_CFG		0x94
+#define DMAC_CFG_PERIPH_EN	BIT(31)
+#define DMAC_CFG_ALLFLUSH_EN	BIT(30)
+#define DMAC_CFG_LASTFLUSH_EN	BIT(29)
+#define DMAC_CFG_QE(x)		(((x) + 1) << 16)
+#define DMAC_CFG_BURST_LEN(x)	(((x) + 1) << 12)
+#define DMAC_CFG_BURST_SZ(x)	((x) << 8)
+#define DMAC_CFG_DIR_READ	BIT(1)
+#define DMAC_CFG_START		BIT(0)
+
+#define DMAC_RD_CNT		0x5c
+#define DMAC_WR_CNT		0x98
+
+#define SDMA_ADDR		0x60
+
+#define DMAM_CFG		0x64
+#define DMAM_CFG_START		BIT(31)
+#define DMAM_CFG_CONT		BIT(30)
+#define DMAM_CFG_SDMA_GAP(x)	(fls((x) / 8192) << 2)
+#define DMAM_CFG_DIR_READ	BIT(1)
+#define DMAM_CFG_EN		BIT(0)
+
+#define DMAM_CNT		0x68
+
+#define LNR_TIMER_TH		0x6c
+
+#define RDM_CFG0		0x78
+#define RDM_CFG0_POLY(x)	(x)
+
+#define RDM_CFG1		0x7c
+#define RDM_CFG1_RDM_EN		BIT(31)
+#define RDM_CFG1_SEED(x)	(x)
+
+#define LWR_SUSP_CTRL		0x90
+#define LWR_SUSP_CTRL_EN	BIT(31)
+
+#define DMAS_CTRL		0x9c
+#define DMAS_CTRL_DIR_READ	BIT(31)
+#define DMAS_CTRL_EN		BIT(30)
+
+#define DATA_STROB		0xa0
+#define DATA_STROB_EDO_EN	BIT(2)
+#define DATA_STROB_INV_POL	BIT(1)
+#define DATA_STROB_DELAY_2CYC	BIT(0)
+
+#define IDLY_CODE(x)		(0xa4 + ((x) * 4))
+#define IDLY_CODE_VAL(x, v)	((v) << (((x) % 4) * 8))
+
+#define GPIO			0xc4
+#define GPIO_PT(x)		BIT(3 + ((x) * 16))
+#define GPIO_RESET(x)		BIT(2 + ((x) * 16))
+#define GPIO_HOLDB(x)		BIT(1 + ((x) * 16))
+#define GPIO_WPB(x)		BIT((x) * 16)
+
+#define HC_VER			0xd0
+
+#define HW_TEST(x)		(0xe0 + ((x) * 4))
+
+struct mx25f0a_mfd {
+	void __iomem *regs;
+	struct clk *ps_clk;
+	struct clk *send_clk;
+	struct clk *send_dly_clk;
+};
+
+#endif // __MFD_MXIC_MX25F0A_H
-- 
1.9.1


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

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

* [PATCH v3 2/4] mtd: rawnand: Add Macronix MX25F0A NAND controller
  2019-04-15  9:23 [PATCH v3 0/4] Add Macronix MX25F0A MFD driver for raw nand and spi Mason Yang
  2019-04-15  9:23 ` [PATCH v3 1/4] mfd: Add Macronix MX25F0A MFD controller driver Mason Yang
@ 2019-04-15  9:23 ` Mason Yang
  2019-05-12 13:18   ` Miquel Raynal
  2019-04-15  9:23 ` [PATCH v3 3/4] spi: Patch Macronix SPI controller driver according to MX25F0A MFD driver Mason Yang
  2019-04-15  9:23 ` [PATCH v3 4/4] dt-bindings: mfd: Document Macronix MX25F0A controller bindings Mason Yang
  3 siblings, 1 reply; 21+ messages in thread
From: Mason Yang @ 2019-04-15  9:23 UTC (permalink / raw)
  To: broonie, marek.vasut, linux-kernel, linux-spi, bbrezillon, dwmw2,
	lee.jones, robh+dt, mark.rutland, computersforpeace, paul.burton,
	stefan, christophe.kerello, liang.yang, geert, devicetree,
	marcel.ziswiler, linux-mtd, richard, miquel.raynal
  Cc: juliensu, Mason Yang, zhengxunli

Add a driver for Macronix MX25F0A NAND controller.

Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
---
 drivers/mtd/nand/raw/Kconfig     |   6 +
 drivers/mtd/nand/raw/Makefile    |   1 +
 drivers/mtd/nand/raw/mxic_nand.c | 294 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 301 insertions(+)
 create mode 100644 drivers/mtd/nand/raw/mxic_nand.c

diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
index e604625..e0329cc 100644
--- a/drivers/mtd/nand/raw/Kconfig
+++ b/drivers/mtd/nand/raw/Kconfig
@@ -522,6 +522,12 @@ config MTD_NAND_QCOM
 	  Enables support for NAND flash chips on SoCs containing the EBI2 NAND
 	  controller. This controller is found on IPQ806x SoC.
 
+config MTD_NAND_MXIC
+	tristate "Macronix MX25F0A NAND controller"
+	depends on HAS_IOMEM
+	help
+	  This selects the Macronix MX25F0A NAND controller driver.
+
 config MTD_NAND_MTK
 	tristate "Support for NAND controller on MTK SoCs"
 	depends on ARCH_MEDIATEK || COMPILE_TEST
diff --git a/drivers/mtd/nand/raw/Makefile b/drivers/mtd/nand/raw/Makefile
index 5a5a72f..c8a6790 100644
--- a/drivers/mtd/nand/raw/Makefile
+++ b/drivers/mtd/nand/raw/Makefile
@@ -54,6 +54,7 @@ obj-$(CONFIG_MTD_NAND_SUNXI)		+= sunxi_nand.o
 obj-$(CONFIG_MTD_NAND_HISI504)	        += hisi504_nand.o
 obj-$(CONFIG_MTD_NAND_BRCMNAND)		+= brcmnand/
 obj-$(CONFIG_MTD_NAND_QCOM)		+= qcom_nandc.o
+obj-$(CONFIG_MTD_NAND_MXIC)		+= mxic_nand.o
 obj-$(CONFIG_MTD_NAND_MTK)		+= mtk_ecc.o mtk_nand.o
 obj-$(CONFIG_MTD_NAND_TEGRA)		+= tegra_nand.o
 obj-$(CONFIG_MTD_NAND_STM32_FMC2)	+= stm32_fmc2_nand.o
diff --git a/drivers/mtd/nand/raw/mxic_nand.c b/drivers/mtd/nand/raw/mxic_nand.c
new file mode 100644
index 0000000..689fae2
--- /dev/null
+++ b/drivers/mtd/nand/raw/mxic_nand.c
@@ -0,0 +1,294 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// Copyright (C) 2019 Macronix International Co., Ltd.
+//
+// Authors:
+//	Mason Yang <masonccyang@mxic.com.tw>
+//	zhengxunli <zhengxunli@mxic.com.tw>
+//
+
+#include <linux/mfd/mxic-mx25f0a.h>
+#include <linux/mtd/rawnand.h>
+#include <linux/mtd/nand_ecc.h>
+
+#include "internals.h"
+
+struct mxic_nand_ctlr {
+	struct mx25f0a_mfd *mfd;
+	struct nand_controller base;
+	struct nand_chip nand;
+};
+
+static void mxic_host_init(struct mxic_nand_ctlr *mxic)
+{
+	writel(DATA_STROB_EDO_EN, mxic->mfd->regs + DATA_STROB);
+	writel(INT_STS_ALL, mxic->mfd->regs + INT_STS_EN);
+	writel(0x0, mxic->mfd->regs + ONFI_DIN_CNT(0));
+	writel(HC_CFG_NIO(8) | HC_CFG_SLV_ACT(0) | HC_CFG_IDLE_SIO_LVL(1) |
+	       HC_CFG_TYPE(1, HC_CFG_TYPE_RAW_NAND) | HC_CFG_MAN_CS_EN,
+	       mxic->mfd->regs + HC_CFG);
+	writel(0x0, mxic->mfd->regs + HC_EN);
+}
+
+static int  mxic_nand_wait_ready(struct nand_chip *chip)
+{
+	struct mxic_nand_ctlr *mxic = nand_get_controller_data(chip);
+	u32 sts;
+
+	return readl_poll_timeout(mxic->mfd->regs + INT_STS, sts,
+				  sts & INT_RDY_PIN, 0, USEC_PER_SEC);
+}
+
+static void mxic_nand_select_chip(struct nand_chip *chip, int chipnr)
+{
+	struct mxic_nand_ctlr *mxic = nand_get_controller_data(chip);
+
+	switch (chipnr) {
+	case 0:
+	case 1:
+		writel(HC_EN_BIT, mxic->mfd->regs + HC_EN);
+		writel(HC_CFG_MAN_CS_ASSERT | readl(mxic->mfd->regs + HC_CFG),
+		       mxic->mfd->regs + HC_CFG);
+		break;
+
+	case -1:
+		writel(~HC_CFG_MAN_CS_ASSERT & readl(mxic->mfd->regs + HC_CFG),
+		       mxic->mfd->regs + HC_CFG);
+		writel(0, mxic->mfd->regs + HC_EN);
+		break;
+
+	default:
+		break;
+	}
+}
+
+static int mxic_nand_data_xfer(struct mxic_nand_ctlr *mxic, const void *txbuf,
+			       void *rxbuf, unsigned int len)
+{
+	unsigned int pos = 0;
+
+	while (pos < len) {
+		unsigned int nbytes = len - pos;
+		u32 data = 0xffffffff;
+		u32 sts;
+		int ret;
+
+		if (nbytes > 4)
+			nbytes = 4;
+
+		if (txbuf)
+			memcpy(&data, txbuf + pos, nbytes);
+
+		ret = readl_poll_timeout(mxic->mfd->regs + INT_STS, sts,
+					 sts & INT_TX_EMPTY, 0, USEC_PER_SEC);
+		if (ret)
+			return ret;
+
+		writel(data, mxic->mfd->regs + TXD(nbytes % 4));
+
+		if (rxbuf) {
+			ret = readl_poll_timeout(mxic->mfd->regs + INT_STS, sts,
+						 sts & INT_TX_EMPTY, 0,
+						 USEC_PER_SEC);
+			if (ret)
+				return ret;
+
+			ret = readl_poll_timeout(mxic->mfd->regs + INT_STS, sts,
+						 sts & INT_RX_NOT_EMPTY, 0,
+						 USEC_PER_SEC);
+			if (ret)
+				return ret;
+
+			data = readl(mxic->mfd->regs + RXD);
+			data >>= (8 * (4 - nbytes));
+			memcpy(rxbuf + pos, &data, nbytes);
+			WARN_ON(readl(mxic->mfd->regs + INT_STS) &
+				INT_RX_NOT_EMPTY);
+		} else {
+			readl(mxic->mfd->regs + RXD);
+		}
+		WARN_ON(readl(mxic->mfd->regs + INT_STS) & INT_RX_NOT_EMPTY);
+
+		pos += nbytes;
+	}
+
+	return 0;
+}
+
+static int mxic_nand_exec_op(struct nand_chip *chip,
+			     const struct nand_operation *op, bool check_only)
+{
+	struct mxic_nand_ctlr *mxic = nand_get_controller_data(chip);
+	const struct nand_op_instr *instr = NULL;
+	int i, len = 0, ret = 0;
+	unsigned int op_id;
+	unsigned char cmdcnt = 0, addr_cnt = 0, cmd_addr[8] = {0};
+
+	for (op_id = 0; op_id < op->ninstrs; op_id++) {
+		instr = &op->instrs[op_id];
+
+		switch (instr->type) {
+		case NAND_OP_CMD_INSTR:
+			cmd_addr[len++] = instr->ctx.cmd.opcode;
+			cmdcnt++;
+			break;
+
+		case NAND_OP_ADDR_INSTR:
+			for (i = 0; i < instr->ctx.addr.naddrs; i++)
+				cmd_addr[len++] = instr->ctx.addr.addrs[i];
+			addr_cnt = i;
+			break;
+
+		case NAND_OP_DATA_IN_INSTR:
+			break;
+
+		case NAND_OP_DATA_OUT_INSTR:
+			writel(instr->ctx.data.len,
+			       mxic->mfd->regs + ONFI_DIN_CNT(0));
+			break;
+
+		case NAND_OP_WAITRDY_INSTR:
+			break;
+		}
+	}
+
+	if (op_id == 5 && instr->type == NAND_OP_WAITRDY_INSTR) {
+		/*
+		 * In case cmd-addr-data-cmd-wait in a sequence,
+		 * separate the 2'nd command, i.e,. nand_prog_page_op()
+		 */
+		writel(OP_CMD_BUSW(OP_BUSW_8) | OP_ADDR_BUSW(OP_BUSW_8) |
+		       OP_DATA_BUSW(OP_BUSW_8) | OP_DUMMY_CYC(0x3F) |
+		       OP_ADDR_BYTES(addr_cnt) |
+		       OP_CMD_BYTES(1), mxic->mfd->regs + SS_CTRL(0));
+		writel(0, mxic->mfd->regs + HC_EN);
+		writel(HC_EN_BIT, mxic->mfd->regs + HC_EN);
+
+		mxic_nand_data_xfer(mxic, cmd_addr, NULL, len - 1);
+
+		mxic_nand_data_xfer(mxic, instr->ctx.data.buf.out, NULL,
+				    instr->ctx.data.len);
+
+		writel(0, mxic->mfd->regs + HC_EN);
+		writel(HC_EN_BIT, mxic->mfd->regs + HC_EN);
+		mxic_nand_data_xfer(mxic, &cmd_addr[--len], NULL, 1);
+		ret = mxic_nand_wait_ready(chip);
+		if (ret)
+			goto err_out;
+		return ret;
+	}
+
+	if (len) {
+		writel(OP_CMD_BUSW(OP_BUSW_8) | OP_ADDR_BUSW(OP_BUSW_8) |
+		       OP_DATA_BUSW(OP_BUSW_8) | OP_DUMMY_CYC(0x3F) |
+		       OP_ADDR_BYTES(addr_cnt) |
+		       OP_CMD_BYTES(cmdcnt > 0 ? cmdcnt : 0),
+		       mxic->mfd->regs + SS_CTRL(0));
+		writel(0, mxic->mfd->regs + HC_EN);
+		writel(HC_EN_BIT, mxic->mfd->regs + HC_EN);
+
+		mxic_nand_data_xfer(mxic, cmd_addr, NULL, len);
+	}
+
+	for (op_id = 0; op_id < op->ninstrs; op_id++) {
+		instr = &op->instrs[op_id];
+
+		switch (instr->type) {
+		case NAND_OP_CMD_INSTR:
+		case NAND_OP_ADDR_INSTR:
+			break;
+
+		case NAND_OP_DATA_IN_INSTR:
+			writel(0x0, mxic->mfd->regs + ONFI_DIN_CNT(0));
+			writel(readl(mxic->mfd->regs + SS_CTRL(0)) | OP_READ,
+			       mxic->mfd->regs + SS_CTRL(0));
+			mxic_nand_data_xfer(mxic, NULL, instr->ctx.data.buf.in,
+					    instr->ctx.data.len);
+			break;
+
+		case NAND_OP_DATA_OUT_INSTR:
+			mxic_nand_data_xfer(mxic, instr->ctx.data.buf.out, NULL,
+					    instr->ctx.data.len);
+			break;
+
+		case NAND_OP_WAITRDY_INSTR:
+			ret = mxic_nand_wait_ready(chip);
+			if (ret)
+				goto err_out;
+			break;
+		}
+	}
+
+err_out:
+	return ret;
+}
+
+static const struct nand_controller_ops mxic_nand_controller_ops = {
+	.exec_op = mxic_nand_exec_op,
+};
+
+static int mx25f0a_nand_probe(struct platform_device *pdev)
+{
+	struct mtd_info *mtd;
+	struct mx25f0a_mfd *mfd = dev_get_drvdata(pdev->dev.parent);
+	struct mxic_nand_ctlr *mxic;
+	struct nand_chip *nand_chip;
+	int err;
+
+	mxic = devm_kzalloc(&pdev->dev, sizeof(struct mxic_nand_ctlr),
+			    GFP_KERNEL);
+	if (!mxic)
+		return -ENOMEM;
+
+	nand_chip = &mxic->nand;
+	mtd = nand_to_mtd(nand_chip);
+	mtd->dev.parent = pdev->dev.parent;
+	nand_chip->ecc.priv = NULL;
+	nand_set_flash_node(nand_chip, pdev->dev.parent->of_node);
+	nand_chip->priv = mxic;
+
+	mxic->mfd = mfd;
+
+	nand_chip->legacy.select_chip = mxic_nand_select_chip;
+	mxic->base.ops = &mxic_nand_controller_ops;
+	nand_controller_init(&mxic->base);
+	nand_chip->controller = &mxic->base;
+
+	mxic_host_init(mxic);
+
+	err = nand_scan(nand_chip, 1);
+	if (err)
+		goto fail;
+
+	err = mtd_device_register(mtd, NULL, 0);
+	if (err)
+		goto fail;
+
+	platform_set_drvdata(pdev, mxic);
+
+	return 0;
+fail:
+	return err;
+}
+
+static int mx25f0a_nand_remove(struct platform_device *pdev)
+{
+	struct mxic_nand_ctlr *mxic = platform_get_drvdata(pdev);
+
+	nand_release(&mxic->nand);
+
+	return 0;
+}
+
+static struct platform_driver mx25f0a_nand_driver = {
+	.probe		= mx25f0a_nand_probe,
+	.remove		= mx25f0a_nand_remove,
+	.driver		= {
+		.name	= "mxic-nand-ctlr",
+	},
+};
+module_platform_driver(mx25f0a_nand_driver);
+
+MODULE_AUTHOR("Mason Yang <masonccyang@mxic.com.tw>");
+MODULE_DESCRIPTION("MX25F0A RAW NAND controller driver");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1


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

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

* [PATCH v3 3/4] spi: Patch Macronix SPI controller driver according to MX25F0A MFD driver
  2019-04-15  9:23 [PATCH v3 0/4] Add Macronix MX25F0A MFD driver for raw nand and spi Mason Yang
  2019-04-15  9:23 ` [PATCH v3 1/4] mfd: Add Macronix MX25F0A MFD controller driver Mason Yang
  2019-04-15  9:23 ` [PATCH v3 2/4] mtd: rawnand: Add Macronix MX25F0A NAND controller Mason Yang
@ 2019-04-15  9:23 ` Mason Yang
  2019-04-19 14:51   ` Mark Brown
  2019-04-15  9:23 ` [PATCH v3 4/4] dt-bindings: mfd: Document Macronix MX25F0A controller bindings Mason Yang
  3 siblings, 1 reply; 21+ messages in thread
From: Mason Yang @ 2019-04-15  9:23 UTC (permalink / raw)
  To: broonie, marek.vasut, linux-kernel, linux-spi, bbrezillon, dwmw2,
	lee.jones, robh+dt, mark.rutland, computersforpeace, paul.burton,
	stefan, christophe.kerello, liang.yang, geert, devicetree,
	marcel.ziswiler, linux-mtd, richard, miquel.raynal
  Cc: juliensu, Mason Yang, zhengxunli

Patch Macronix MX25F0A SPI controller driver according to it's MFD driver.

Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
---
 drivers/spi/spi-mxic.c | 275 +++++++++----------------------------------------
 1 file changed, 49 insertions(+), 226 deletions(-)

diff --git a/drivers/spi/spi-mxic.c b/drivers/spi/spi-mxic.c
index e41ae6e..f98f8e0 100644
--- a/drivers/spi/spi-mxic.c
+++ b/drivers/spi/spi-mxic.c
@@ -9,168 +9,13 @@
 //
 
 #include <linux/clk.h>
-#include <linux/io.h>
-#include <linux/iopoll.h>
-#include <linux/module.h>
-#include <linux/platform_device.h>
+#include <linux/mfd/mxic-mx25f0a.h>
 #include <linux/pm_runtime.h>
 #include <linux/spi/spi.h>
 #include <linux/spi/spi-mem.h>
 
-#define HC_CFG			0x0
-#define HC_CFG_IF_CFG(x)	((x) << 27)
-#define HC_CFG_DUAL_SLAVE	BIT(31)
-#define HC_CFG_INDIVIDUAL	BIT(30)
-#define HC_CFG_NIO(x)		(((x) / 4) << 27)
-#define HC_CFG_TYPE(s, t)	((t) << (23 + ((s) * 2)))
-#define HC_CFG_TYPE_SPI_NOR	0
-#define HC_CFG_TYPE_SPI_NAND	1
-#define HC_CFG_TYPE_SPI_RAM	2
-#define HC_CFG_TYPE_RAW_NAND	3
-#define HC_CFG_SLV_ACT(x)	((x) << 21)
-#define HC_CFG_CLK_PH_EN	BIT(20)
-#define HC_CFG_CLK_POL_INV	BIT(19)
-#define HC_CFG_BIG_ENDIAN	BIT(18)
-#define HC_CFG_DATA_PASS	BIT(17)
-#define HC_CFG_IDLE_SIO_LVL(x)	((x) << 16)
-#define HC_CFG_MAN_START_EN	BIT(3)
-#define HC_CFG_MAN_START	BIT(2)
-#define HC_CFG_MAN_CS_EN	BIT(1)
-#define HC_CFG_MAN_CS_ASSERT	BIT(0)
-
-#define INT_STS			0x4
-#define INT_STS_EN		0x8
-#define INT_SIG_EN		0xc
-#define INT_STS_ALL		GENMASK(31, 0)
-#define INT_RDY_PIN		BIT(26)
-#define INT_RDY_SR		BIT(25)
-#define INT_LNR_SUSP		BIT(24)
-#define INT_ECC_ERR		BIT(17)
-#define INT_CRC_ERR		BIT(16)
-#define INT_LWR_DIS		BIT(12)
-#define INT_LRD_DIS		BIT(11)
-#define INT_SDMA_INT		BIT(10)
-#define INT_DMA_FINISH		BIT(9)
-#define INT_RX_NOT_FULL		BIT(3)
-#define INT_RX_NOT_EMPTY	BIT(2)
-#define INT_TX_NOT_FULL		BIT(1)
-#define INT_TX_EMPTY		BIT(0)
-
-#define HC_EN			0x10
-#define HC_EN_BIT		BIT(0)
-
-#define TXD(x)			(0x14 + ((x) * 4))
-#define RXD			0x24
-
-#define SS_CTRL(s)		(0x30 + ((s) * 4))
-#define LRD_CFG			0x44
-#define LWR_CFG			0x80
-#define RWW_CFG			0x70
-#define OP_READ			BIT(23)
-#define OP_DUMMY_CYC(x)		((x) << 17)
-#define OP_ADDR_BYTES(x)	((x) << 14)
-#define OP_CMD_BYTES(x)		(((x) - 1) << 13)
-#define OP_OCTA_CRC_EN		BIT(12)
-#define OP_DQS_EN		BIT(11)
-#define OP_ENHC_EN		BIT(10)
-#define OP_PREAMBLE_EN		BIT(9)
-#define OP_DATA_DDR		BIT(8)
-#define OP_DATA_BUSW(x)		((x) << 6)
-#define OP_ADDR_DDR		BIT(5)
-#define OP_ADDR_BUSW(x)		((x) << 3)
-#define OP_CMD_DDR		BIT(2)
-#define OP_CMD_BUSW(x)		(x)
-#define OP_BUSW_1		0
-#define OP_BUSW_2		1
-#define OP_BUSW_4		2
-#define OP_BUSW_8		3
-
-#define OCTA_CRC		0x38
-#define OCTA_CRC_IN_EN(s)	BIT(3 + ((s) * 16))
-#define OCTA_CRC_CHUNK(s, x)	((fls((x) / 32)) << (1 + ((s) * 16)))
-#define OCTA_CRC_OUT_EN(s)	BIT(0 + ((s) * 16))
-
-#define ONFI_DIN_CNT(s)		(0x3c + (s))
-
-#define LRD_CTRL		0x48
-#define RWW_CTRL		0x74
-#define LWR_CTRL		0x84
-#define LMODE_EN		BIT(31)
-#define LMODE_SLV_ACT(x)	((x) << 21)
-#define LMODE_CMD1(x)		((x) << 8)
-#define LMODE_CMD0(x)		(x)
-
-#define LRD_ADDR		0x4c
-#define LWR_ADDR		0x88
-#define LRD_RANGE		0x50
-#define LWR_RANGE		0x8c
-
-#define AXI_SLV_ADDR		0x54
-
-#define DMAC_RD_CFG		0x58
-#define DMAC_WR_CFG		0x94
-#define DMAC_CFG_PERIPH_EN	BIT(31)
-#define DMAC_CFG_ALLFLUSH_EN	BIT(30)
-#define DMAC_CFG_LASTFLUSH_EN	BIT(29)
-#define DMAC_CFG_QE(x)		(((x) + 1) << 16)
-#define DMAC_CFG_BURST_LEN(x)	(((x) + 1) << 12)
-#define DMAC_CFG_BURST_SZ(x)	((x) << 8)
-#define DMAC_CFG_DIR_READ	BIT(1)
-#define DMAC_CFG_START		BIT(0)
-
-#define DMAC_RD_CNT		0x5c
-#define DMAC_WR_CNT		0x98
-
-#define SDMA_ADDR		0x60
-
-#define DMAM_CFG		0x64
-#define DMAM_CFG_START		BIT(31)
-#define DMAM_CFG_CONT		BIT(30)
-#define DMAM_CFG_SDMA_GAP(x)	(fls((x) / 8192) << 2)
-#define DMAM_CFG_DIR_READ	BIT(1)
-#define DMAM_CFG_EN		BIT(0)
-
-#define DMAM_CNT		0x68
-
-#define LNR_TIMER_TH		0x6c
-
-#define RDM_CFG0		0x78
-#define RDM_CFG0_POLY(x)	(x)
-
-#define RDM_CFG1		0x7c
-#define RDM_CFG1_RDM_EN		BIT(31)
-#define RDM_CFG1_SEED(x)	(x)
-
-#define LWR_SUSP_CTRL		0x90
-#define LWR_SUSP_CTRL_EN	BIT(31)
-
-#define DMAS_CTRL		0x9c
-#define DMAS_CTRL_DIR_READ	BIT(31)
-#define DMAS_CTRL_EN		BIT(30)
-
-#define DATA_STROB		0xa0
-#define DATA_STROB_EDO_EN	BIT(2)
-#define DATA_STROB_INV_POL	BIT(1)
-#define DATA_STROB_DELAY_2CYC	BIT(0)
-
-#define IDLY_CODE(x)		(0xa4 + ((x) * 4))
-#define IDLY_CODE_VAL(x, v)	((v) << (((x) % 4) * 8))
-
-#define GPIO			0xc4
-#define GPIO_PT(x)		BIT(3 + ((x) * 16))
-#define GPIO_RESET(x)		BIT(2 + ((x) * 16))
-#define GPIO_HOLDB(x)		BIT(1 + ((x) * 16))
-#define GPIO_WPB(x)		BIT((x) * 16)
-
-#define HC_VER			0xd0
-
-#define HW_TEST(x)		(0xe0 + ((x) * 4))
-
 struct mxic_spi {
-	struct clk *ps_clk;
-	struct clk *send_clk;
-	struct clk *send_dly_clk;
-	void __iomem *regs;
+	struct mx25f0a_mfd *mfd;
 	u32 cur_speed_hz;
 };
 
@@ -178,26 +23,26 @@ static int mxic_spi_clk_enable(struct mxic_spi *mxic)
 {
 	int ret;
 
-	ret = clk_prepare_enable(mxic->send_clk);
+	ret = clk_prepare_enable(mxic->mfd->send_clk);
 	if (ret)
 		return ret;
 
-	ret = clk_prepare_enable(mxic->send_dly_clk);
+	ret = clk_prepare_enable(mxic->mfd->send_dly_clk);
 	if (ret)
 		goto err_send_dly_clk;
 
 	return ret;
 
 err_send_dly_clk:
-	clk_disable_unprepare(mxic->send_clk);
+	clk_disable_unprepare(mxic->mfd->send_clk);
 
 	return ret;
 }
 
 static void mxic_spi_clk_disable(struct mxic_spi *mxic)
 {
-	clk_disable_unprepare(mxic->send_clk);
-	clk_disable_unprepare(mxic->send_dly_clk);
+	clk_disable_unprepare(mxic->mfd->send_clk);
+	clk_disable_unprepare(mxic->mfd->send_dly_clk);
 }
 
 static void mxic_spi_set_input_delay_dqs(struct mxic_spi *mxic, u8 idly_code)
@@ -206,23 +51,23 @@ static void mxic_spi_set_input_delay_dqs(struct mxic_spi *mxic, u8 idly_code)
 	       IDLY_CODE_VAL(1, idly_code) |
 	       IDLY_CODE_VAL(2, idly_code) |
 	       IDLY_CODE_VAL(3, idly_code),
-	       mxic->regs + IDLY_CODE(0));
+	       mxic->mfd->regs + IDLY_CODE(0));
 	writel(IDLY_CODE_VAL(4, idly_code) |
 	       IDLY_CODE_VAL(5, idly_code) |
 	       IDLY_CODE_VAL(6, idly_code) |
 	       IDLY_CODE_VAL(7, idly_code),
-	       mxic->regs + IDLY_CODE(1));
+	       mxic->mfd->regs + IDLY_CODE(1));
 }
 
 static int mxic_spi_clk_setup(struct mxic_spi *mxic, unsigned long freq)
 {
 	int ret;
 
-	ret = clk_set_rate(mxic->send_clk, freq);
+	ret = clk_set_rate(mxic->mfd->send_clk, freq);
 	if (ret)
 		return ret;
 
-	ret = clk_set_rate(mxic->send_dly_clk, freq);
+	ret = clk_set_rate(mxic->mfd->send_dly_clk, freq);
 	if (ret)
 		return ret;
 
@@ -240,7 +85,7 @@ static int mxic_spi_clk_setup(struct mxic_spi *mxic, unsigned long freq)
 	 *                  = 360 * freq * 1 sec / 1000000000
 	 *                  = 9 * freq / 25000000
 	 */
-	ret = clk_set_phase(mxic->send_dly_clk, 9 * freq / 25000000);
+	ret = clk_set_phase(mxic->mfd->send_dly_clk, 9 * freq / 25000000);
 	if (ret)
 		return ret;
 
@@ -270,14 +115,14 @@ static int mxic_spi_set_freq(struct mxic_spi *mxic, unsigned long freq)
 
 static void mxic_spi_hw_init(struct mxic_spi *mxic)
 {
-	writel(0, mxic->regs + DATA_STROB);
-	writel(INT_STS_ALL, mxic->regs + INT_STS_EN);
-	writel(0, mxic->regs + HC_EN);
-	writel(0, mxic->regs + LRD_CFG);
-	writel(0, mxic->regs + LRD_CTRL);
+	writel(0, mxic->mfd->regs + DATA_STROB);
+	writel(INT_STS_ALL, mxic->mfd->regs + INT_STS_EN);
+	writel(0, mxic->mfd->regs + HC_EN);
+	writel(0, mxic->mfd->regs + LRD_CFG);
+	writel(0, mxic->mfd->regs + LRD_CTRL);
 	writel(HC_CFG_NIO(1) | HC_CFG_TYPE(0, HC_CFG_TYPE_SPI_NAND) |
 	       HC_CFG_SLV_ACT(0) | HC_CFG_MAN_CS_EN | HC_CFG_IDLE_SIO_LVL(1),
-	       mxic->regs + HC_CFG);
+	       mxic->mfd->regs + HC_CFG);
 }
 
 static int mxic_spi_data_xfer(struct mxic_spi *mxic, const void *txbuf,
@@ -297,34 +142,35 @@ static int mxic_spi_data_xfer(struct mxic_spi *mxic, const void *txbuf,
 		if (txbuf)
 			memcpy(&data, txbuf + pos, nbytes);
 
-		ret = readl_poll_timeout(mxic->regs + INT_STS, sts,
+		ret = readl_poll_timeout(mxic->mfd->regs + INT_STS, sts,
 					 sts & INT_TX_EMPTY, 0, USEC_PER_SEC);
 		if (ret)
 			return ret;
 
-		writel(data, mxic->regs + TXD(nbytes % 4));
+		writel(data, mxic->mfd->regs + TXD(nbytes % 4));
 
 		if (rxbuf) {
-			ret = readl_poll_timeout(mxic->regs + INT_STS, sts,
+			ret = readl_poll_timeout(mxic->mfd->regs + INT_STS, sts,
 						 sts & INT_TX_EMPTY, 0,
 						 USEC_PER_SEC);
 			if (ret)
 				return ret;
 
-			ret = readl_poll_timeout(mxic->regs + INT_STS, sts,
+			ret = readl_poll_timeout(mxic->mfd->regs + INT_STS, sts,
 						 sts & INT_RX_NOT_EMPTY, 0,
 						 USEC_PER_SEC);
 			if (ret)
 				return ret;
 
-			data = readl(mxic->regs + RXD);
+			data = readl(mxic->mfd->regs + RXD);
 			data >>= (8 * (4 - nbytes));
 			memcpy(rxbuf + pos, &data, nbytes);
-			WARN_ON(readl(mxic->regs + INT_STS) & INT_RX_NOT_EMPTY);
+			WARN_ON(readl(mxic->mfd->regs + INT_STS) &
+				INT_RX_NOT_EMPTY);
 		} else {
-			readl(mxic->regs + RXD);
+			readl(mxic->mfd->regs + RXD);
 		}
-		WARN_ON(readl(mxic->regs + INT_STS) & INT_RX_NOT_EMPTY);
+		WARN_ON(readl(mxic->mfd->regs + INT_STS) & INT_RX_NOT_EMPTY);
 
 		pos += nbytes;
 	}
@@ -370,8 +216,8 @@ static int mxic_spi_mem_exec_op(struct spi_mem *mem,
 	       HC_CFG_TYPE(mem->spi->chip_select, HC_CFG_TYPE_SPI_NOR) |
 	       HC_CFG_SLV_ACT(mem->spi->chip_select) | HC_CFG_IDLE_SIO_LVL(1) |
 	       HC_CFG_MAN_CS_EN,
-	       mxic->regs + HC_CFG);
-	writel(HC_EN_BIT, mxic->regs + HC_EN);
+	       mxic->mfd->regs + HC_CFG);
+	writel(HC_EN_BIT, mxic->mfd->regs + HC_EN);
 
 	ss_ctrl = OP_CMD_BYTES(1) | OP_CMD_BUSW(fls(op->cmd.buswidth) - 1);
 
@@ -388,10 +234,10 @@ static int mxic_spi_mem_exec_op(struct spi_mem *mem,
 			ss_ctrl |= OP_READ;
 	}
 
-	writel(ss_ctrl, mxic->regs + SS_CTRL(mem->spi->chip_select));
+	writel(ss_ctrl, mxic->mfd->regs + SS_CTRL(mem->spi->chip_select));
 
-	writel(readl(mxic->regs + HC_CFG) | HC_CFG_MAN_CS_ASSERT,
-	       mxic->regs + HC_CFG);
+	writel(readl(mxic->mfd->regs + HC_CFG) | HC_CFG_MAN_CS_ASSERT,
+	       mxic->mfd->regs + HC_CFG);
 
 	ret = mxic_spi_data_xfer(mxic, &op->cmd.opcode, NULL, 1);
 	if (ret)
@@ -416,9 +262,9 @@ static int mxic_spi_mem_exec_op(struct spi_mem *mem,
 				 op->data.nbytes);
 
 out:
-	writel(readl(mxic->regs + HC_CFG) & ~HC_CFG_MAN_CS_ASSERT,
-	       mxic->regs + HC_CFG);
-	writel(0, mxic->regs + HC_EN);
+	writel(readl(mxic->mfd->regs + HC_CFG) & ~HC_CFG_MAN_CS_ASSERT,
+	       mxic->mfd->regs + HC_CFG);
+	writel(0, mxic->mfd->regs + HC_EN);
 
 	return ret;
 }
@@ -433,15 +279,15 @@ static void mxic_spi_set_cs(struct spi_device *spi, bool lvl)
 	struct mxic_spi *mxic = spi_master_get_devdata(spi->master);
 
 	if (!lvl) {
-		writel(readl(mxic->regs + HC_CFG) | HC_CFG_MAN_CS_EN,
-		       mxic->regs + HC_CFG);
-		writel(HC_EN_BIT, mxic->regs + HC_EN);
-		writel(readl(mxic->regs + HC_CFG) | HC_CFG_MAN_CS_ASSERT,
-		       mxic->regs + HC_CFG);
+		writel(readl(mxic->mfd->regs + HC_CFG) | HC_CFG_MAN_CS_EN,
+		       mxic->mfd->regs + HC_CFG);
+		writel(HC_EN_BIT, mxic->mfd->regs + HC_EN);
+		writel(readl(mxic->mfd->regs + HC_CFG) | HC_CFG_MAN_CS_ASSERT,
+		       mxic->mfd->regs + HC_CFG);
 	} else {
-		writel(readl(mxic->regs + HC_CFG) & ~HC_CFG_MAN_CS_ASSERT,
-		       mxic->regs + HC_CFG);
-		writel(0, mxic->regs + HC_EN);
+		writel(readl(mxic->mfd->regs + HC_CFG) & ~HC_CFG_MAN_CS_ASSERT,
+		       mxic->mfd->regs + HC_CFG);
+		writel(0, mxic->mfd->regs + HC_EN);
 	}
 }
 
@@ -479,7 +325,7 @@ static int mxic_spi_transfer_one(struct spi_master *master,
 
 	writel(OP_CMD_BYTES(1) | OP_CMD_BUSW(busw) |
 	       OP_DATA_BUSW(busw) | (t->rx_buf ? OP_READ : 0),
-	       mxic->regs + SS_CTRL(0));
+	       mxic->mfd->regs + SS_CTRL(0));
 
 	ret = mxic_spi_data_xfer(mxic, t->tx_buf, t->rx_buf, t->len);
 	if (ret)
@@ -497,7 +343,7 @@ static int __maybe_unused mxic_spi_runtime_suspend(struct device *dev)
 	struct mxic_spi *mxic = spi_master_get_devdata(master);
 
 	mxic_spi_clk_disable(mxic);
-	clk_disable_unprepare(mxic->ps_clk);
+	clk_disable_unprepare(mxic->mfd->ps_clk);
 
 	return 0;
 }
@@ -509,7 +355,7 @@ static int __maybe_unused mxic_spi_runtime_resume(struct device *dev)
 	struct mxic_spi *mxic = spi_master_get_devdata(master);
 	int ret;
 
-	ret = clk_prepare_enable(mxic->ps_clk);
+	ret = clk_prepare_enable(mxic->mfd->ps_clk);
 	if (ret) {
 		dev_err(dev, "Cannot enable ps_clock.\n");
 		return ret;
@@ -525,8 +371,8 @@ static int __maybe_unused mxic_spi_runtime_resume(struct device *dev)
 
 static int mxic_spi_probe(struct platform_device *pdev)
 {
+	struct mx25f0a_mfd *mfd = dev_get_drvdata(pdev->dev.parent);
 	struct spi_master *master;
-	struct resource *res;
 	struct mxic_spi *mxic;
 	int ret;
 
@@ -538,24 +384,8 @@ static int mxic_spi_probe(struct platform_device *pdev)
 
 	mxic = spi_master_get_devdata(master);
 
-	master->dev.of_node = pdev->dev.of_node;
-
-	mxic->ps_clk = devm_clk_get(&pdev->dev, "ps_clk");
-	if (IS_ERR(mxic->ps_clk))
-		return PTR_ERR(mxic->ps_clk);
-
-	mxic->send_clk = devm_clk_get(&pdev->dev, "send_clk");
-	if (IS_ERR(mxic->send_clk))
-		return PTR_ERR(mxic->send_clk);
-
-	mxic->send_dly_clk = devm_clk_get(&pdev->dev, "send_dly_clk");
-	if (IS_ERR(mxic->send_dly_clk))
-		return PTR_ERR(mxic->send_dly_clk);
-
-	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
-	mxic->regs = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(mxic->regs))
-		return PTR_ERR(mxic->regs);
+	master->dev.of_node = pdev->dev.parent->of_node;
+	mxic->mfd = mfd;
 
 	pm_runtime_enable(&pdev->dev);
 	master->auto_runtime_pm = true;
@@ -597,18 +427,11 @@ static int mxic_spi_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static const struct of_device_id mxic_spi_of_ids[] = {
-	{ .compatible = "mxicy,mx25f0a-spi", },
-	{ /* sentinel */ }
-};
-MODULE_DEVICE_TABLE(of, mxic_spi_of_ids);
-
 static struct platform_driver mxic_spi_driver = {
 	.probe = mxic_spi_probe,
 	.remove = mxic_spi_remove,
 	.driver = {
 		.name = "mxic-spi",
-		.of_match_table = mxic_spi_of_ids,
 		.pm = &mxic_spi_dev_pm_ops,
 	},
 };
-- 
1.9.1


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

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

* [PATCH v3 4/4] dt-bindings: mfd: Document Macronix MX25F0A controller bindings
  2019-04-15  9:23 [PATCH v3 0/4] Add Macronix MX25F0A MFD driver for raw nand and spi Mason Yang
                   ` (2 preceding siblings ...)
  2019-04-15  9:23 ` [PATCH v3 3/4] spi: Patch Macronix SPI controller driver according to MX25F0A MFD driver Mason Yang
@ 2019-04-15  9:23 ` Mason Yang
  2019-04-26 22:41   ` Rob Herring
  2019-05-12 13:23   ` Miquel Raynal
  3 siblings, 2 replies; 21+ messages in thread
From: Mason Yang @ 2019-04-15  9:23 UTC (permalink / raw)
  To: broonie, marek.vasut, linux-kernel, linux-spi, bbrezillon, dwmw2,
	lee.jones, robh+dt, mark.rutland, computersforpeace, paul.burton,
	stefan, christophe.kerello, liang.yang, geert, devicetree,
	marcel.ziswiler, linux-mtd, richard, miquel.raynal
  Cc: juliensu, Mason Yang, zhengxunli

Document the bindings used by the Macronix MX25F0A MFD controller.

Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
---
 .../devicetree/bindings/mfd/mxic-mx25f0a.txt       | 51 ++++++++++++++++++++++
 1 file changed, 51 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/mxic-mx25f0a.txt

diff --git a/Documentation/devicetree/bindings/mfd/mxic-mx25f0a.txt b/Documentation/devicetree/bindings/mfd/mxic-mx25f0a.txt
new file mode 100644
index 0000000..7f3e0f8
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/mxic-mx25f0a.txt
@@ -0,0 +1,51 @@
+Macronix MX25F0A MultiFunction Device Tree Bindings
+----------------------------------------------------
+
+MX25F0A is a MultiFunction Device with SPI and raw NAND, which
+supports either spi host controller or raw nand controller.
+
+Required properties:
+- compatible: should be "mxic,mx25f0a"
+- #address-cells: should be 1
+- #size-cells: should be 0
+- reg: should contain 2 entries, one for the registers and one for the direct
+       mapping area in SPI mode.
+- reg-names: should contain "regs" and "dirmap"
+- interrupts: interrupt line connected to this MFD controller
+- SPI controller driver:
+		- clock-names: should contain "ps_clk", "send_clk" and
+			       "send_dly_clk"
+		- clocks: should contain 3 entries for the "ps_clk", "send_clk"
+			  and "send_dly_clk" clocks
+
+- Raw nand controller driver.
+		- nand-ecc-mode = "soft";
+		- nand-ecc-algo = "bch";
+
+Example:
+
+	mxic: mx25f0a@43c30000 {
+		compatible = "mxic,mx25f0a";
+		reg = <0x43c30000 0x10000>, <0xa0000000 0x4000000>;
+		reg-names = "regs", "dirmap";
+
+		/* spi */
+		clocks = <&clkwizard 0>, <&clkwizard 1>, <&clkc 15>;
+		clock-names = "send_clk", "send_dly_clk", "ps_clk";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		flash@0 {
+			compatible = "jedec,spi-nor";
+			reg = <0>;
+			spi-max-frequency = <25000000>;
+			spi-tx-bus-width = <4>;
+			spi-rx-bus-width = <4>;
+		};
+
+		/* nand */
+		nand-ecc-mode = "soft";
+		nand-ecc-algo = "bch";
+		nand-ecc-step-size = <512>;
+		nand-ecc-strength = <8>;
+	};
-- 
1.9.1


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

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

* Re: [PATCH v3 3/4] spi: Patch Macronix SPI controller driver according to MX25F0A MFD driver
  2019-04-15  9:23 ` [PATCH v3 3/4] spi: Patch Macronix SPI controller driver according to MX25F0A MFD driver Mason Yang
@ 2019-04-19 14:51   ` Mark Brown
       [not found]     ` <OF7742B4A9.445066F6-ON482583EC.0037E377-482583EC.0039125B@mxic.com.tw>
  0 siblings, 1 reply; 21+ messages in thread
From: Mark Brown @ 2019-04-19 14:51 UTC (permalink / raw)
  To: Mason Yang
  Cc: mark.rutland, devicetree, juliensu, christophe.kerello, richard,
	bbrezillon, marcel.ziswiler, dwmw2, miquel.raynal, linux-kernel,
	stefan, linux-spi, marek.vasut, paul.burton, robh+dt, geert,
	linux-mtd, computersforpeace, liang.yang, lee.jones, zhengxunli

[-- Attachment #1.1: Type: text/plain, Size: 776 bytes --]

On Mon, Apr 15, 2019 at 05:23:53PM +0800, Mason Yang wrote:
> Patch Macronix MX25F0A SPI controller driver according to it's MFD driver.

It'd be much better to describe what the above actually means - what
changes have been made in the introduction of the MFD driver?  It does
feel like there's not as much abstraction as I'd expect between the MFD
and the child, there's a lot of peering into the parent and enabling and
disabling individual clocks for example rather than either having this
hidden behind a function or just having the clocks owned by the child
driver.  The driver also isn't using the MFD interfaces to pass through
the register subblocks for the IP - instead the child driver is peering
straight into the MFD structure and looking at a variable in there.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

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

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

* Re: [PATCH v3 4/4] dt-bindings: mfd: Document Macronix MX25F0A controller bindings
  2019-04-15  9:23 ` [PATCH v3 4/4] dt-bindings: mfd: Document Macronix MX25F0A controller bindings Mason Yang
@ 2019-04-26 22:41   ` Rob Herring
  2019-05-12 13:23   ` Miquel Raynal
  1 sibling, 0 replies; 21+ messages in thread
From: Rob Herring @ 2019-04-26 22:41 UTC (permalink / raw)
  To: Mason Yang
  Cc: mark.rutland, devicetree, juliensu, christophe.kerello, richard,
	bbrezillon, marcel.ziswiler, lee.jones, miquel.raynal,
	linux-kernel, stefan, linux-spi, marek.vasut, paul.burton,
	broonie, geert, linux-mtd, computersforpeace, liang.yang, dwmw2,
	zhengxunli

On Mon, Apr 15, 2019 at 05:23:54PM +0800, Mason Yang wrote:
> Document the bindings used by the Macronix MX25F0A MFD controller.
> 
> Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
> ---
>  .../devicetree/bindings/mfd/mxic-mx25f0a.txt       | 51 ++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/mxic-mx25f0a.txt
> 
> diff --git a/Documentation/devicetree/bindings/mfd/mxic-mx25f0a.txt b/Documentation/devicetree/bindings/mfd/mxic-mx25f0a.txt
> new file mode 100644
> index 0000000..7f3e0f8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/mxic-mx25f0a.txt
> @@ -0,0 +1,51 @@
> +Macronix MX25F0A MultiFunction Device Tree Bindings
> +----------------------------------------------------
> +
> +MX25F0A is a MultiFunction Device with SPI and raw NAND, which
> +supports either spi host controller or raw nand controller.
> +
> +Required properties:
> +- compatible: should be "mxic,mx25f0a"
> +- #address-cells: should be 1
> +- #size-cells: should be 0
> +- reg: should contain 2 entries, one for the registers and one for the direct
> +       mapping area in SPI mode.
> +- reg-names: should contain "regs" and "dirmap"
> +- interrupts: interrupt line connected to this MFD controller
> +- SPI controller driver:
> +		- clock-names: should contain "ps_clk", "send_clk" and
> +			       "send_dly_clk"
> +		- clocks: should contain 3 entries for the "ps_clk", "send_clk"
> +			  and "send_dly_clk" clocks
> +
> +- Raw nand controller driver.
> +		- nand-ecc-mode = "soft";
> +		- nand-ecc-algo = "bch";
> +
> +Example:
> +
> +	mxic: mx25f0a@43c30000 {
> +		compatible = "mxic,mx25f0a";
> +		reg = <0x43c30000 0x10000>, <0xa0000000 0x4000000>;
> +		reg-names = "regs", "dirmap";
> +
> +		/* spi */
> +		clocks = <&clkwizard 0>, <&clkwizard 1>, <&clkc 15>;
> +		clock-names = "send_clk", "send_dly_clk", "ps_clk";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		flash@0 {
> +			compatible = "jedec,spi-nor";
> +			reg = <0>;
> +			spi-max-frequency = <25000000>;
> +			spi-tx-bus-width = <4>;
> +			spi-rx-bus-width = <4>;
> +		};
> +
> +		/* nand */

Don't you need a nand child node? I'm not sure how that's going to work 
as you are already using the number space (i.e. reg) for SPI CS number 
and you can't really mix number spaces within a node level.

> +		nand-ecc-mode = "soft";
> +		nand-ecc-algo = "bch";
> +		nand-ecc-step-size = <512>;
> +		nand-ecc-strength = <8>;
> +	};
> -- 
> 1.9.1
> 

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

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

* Re: [PATCH v3 3/4] spi: Patch Macronix SPI controller driver according to MX25F0A MFD driver
       [not found]     ` <OF7742B4A9.445066F6-ON482583EC.0037E377-482583EC.0039125B@mxic.com.tw>
@ 2019-05-02  2:41       ` Mark Brown
  0 siblings, 0 replies; 21+ messages in thread
From: Mark Brown @ 2019-05-02  2:41 UTC (permalink / raw)
  To: masonccyang
  Cc: mark.rutland, devicetree, christophe.kerello, richard,
	bbrezillon, juliensu, computersforpeace, linux-kernel, robh+dt,
	linux-spi, marcel.ziswiler, paul.burton, liang.yang, geert,
	stefan, miquel.raynal, linux-mtd, lee.jones, dwmw2, marek.vasut,
	zhengxunli

[-- Attachment #1.1: Type: text/plain, Size: 1462 bytes --]

On Tue, Apr 30, 2019 at 06:23:21PM +0800, masonccyang@mxic.com.tw wrote:

> > It'd be much better to describe what the above actually means - what
> > changes have been made in the introduction of the MFD driver?  It does
> > feel like there's not as much abstraction as I'd expect between the MFD
> > and the child, there's a lot of peering into the parent and enabling and
> > disabling individual clocks for example rather than either having this
> > hidden behind a function or just having the clocks owned by the child
> > driver. 

> Do you mean I should remove ps_clk/send_clk/send_dly_clk resource from MFD 

> and patch them to spi-mxic.c ?

> Or any other ?

I think you need to have a clear idea that you can explain as to what
the MFD is and how it's split up.  What's being abstracted, what's not
and why.  Without knowing anything about the device or what the series
is trying to accomplish it's hard to be sure exactly what the best thing
to do is.

> The driver also isn't using the MFD interfaces to pass through
> > the register subblocks for the IP - instead the child driver is peering
> > straight into the MFD structure and looking at a variable in there.

> Patch regmap for mfd, nand and spi ?
> or any other patches is needed ?

This is a memory mapped device so there should be no need to use regmap
unless you want to.  The MFD subsystem has facilities for passing
through memory regions to subdevices.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

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

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

* Re: [PATCH v3 1/4]  mfd: Add Macronix MX25F0A MFD controller driver
  2019-04-15  9:23 ` [PATCH v3 1/4] mfd: Add Macronix MX25F0A MFD controller driver Mason Yang
@ 2019-05-12 13:08   ` Miquel Raynal
  2019-05-15  6:46     ` masonccyang
  0 siblings, 1 reply; 21+ messages in thread
From: Miquel Raynal @ 2019-05-12 13:08 UTC (permalink / raw)
  To: Mason Yang
  Cc: mark.rutland, devicetree, christophe.kerello, juliensu,
	bbrezillon, marcel.ziswiler, lee.jones, linux-kernel, robh+dt,
	linux-spi, marek.vasut, paul.burton, broonie, geert, stefan,
	linux-mtd, richard, liang.yang, computersforpeace, dwmw2,
	zhengxunli

Hi Mason,

Mason Yang <masonccyang@mxic.com.tw> wrote on Mon, 15 Apr 2019 17:23:51
+0800:

> Add a driver for Macronix MX25F0A multifunction device controller.
> 
> Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
> ---
>  drivers/mfd/Kconfig              |   9 ++
>  drivers/mfd/Makefile             |   1 +
>  drivers/mfd/mxic-mx25f0a.c       |  84 +++++++++++++++++++
>  include/linux/mfd/mxic-mx25f0a.h | 175 +++++++++++++++++++++++++++++++++++++++
>  4 files changed, 269 insertions(+)
>  create mode 100644 drivers/mfd/mxic-mx25f0a.c
>  create mode 100644 include/linux/mfd/mxic-mx25f0a.h
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 26ad646..7e99e93 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -823,6 +823,15 @@ config MFD_MAX8998
>  	  additional drivers must be enabled in order to use the functionality
>  	  of the device.
>  
> +config MFD_MXIC_MX25F0A
> +	tristate "Macronix mx25f0a multifunction device support"
> +	select MFD_CORE
> +	help
> +	  This supports for Macronix mx25f0a multifunction device controller
> +	  for raw nand or spi. You have to select individual components like

Please use upper case for acronyms in plain English: NAND, SPI

> +	  raw nand controller or spi host controller under the corresponding
> +	  menus.
> +
>  config MFD_MT6397
>  	tristate "MediaTek MT6397 PMIC Support"
>  	select MFD_CORE

Thanks,
Miquèl

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

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

* Re: [PATCH v3 2/4] mtd: rawnand: Add Macronix MX25F0A NAND controller
  2019-04-15  9:23 ` [PATCH v3 2/4] mtd: rawnand: Add Macronix MX25F0A NAND controller Mason Yang
@ 2019-05-12 13:18   ` Miquel Raynal
  2019-05-15  8:48     ` masonccyang
                       ` (2 more replies)
  0 siblings, 3 replies; 21+ messages in thread
From: Miquel Raynal @ 2019-05-12 13:18 UTC (permalink / raw)
  To: Mason Yang
  Cc: mark.rutland, devicetree, christophe.kerello, juliensu,
	bbrezillon, marcel.ziswiler, lee.jones, linux-kernel, robh+dt,
	linux-spi, marek.vasut, paul.burton, broonie, geert, stefan,
	linux-mtd, richard, liang.yang, computersforpeace, dwmw2,
	zhengxunli

Hi Mason,

Mason Yang <masonccyang@mxic.com.tw> wrote on Mon, 15 Apr 2019 17:23:52
+0800:

> Add a driver for Macronix MX25F0A NAND controller.
> 
> Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
> ---
>  drivers/mtd/nand/raw/Kconfig     |   6 +
>  drivers/mtd/nand/raw/Makefile    |   1 +
>  drivers/mtd/nand/raw/mxic_nand.c | 294 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 301 insertions(+)
>  create mode 100644 drivers/mtd/nand/raw/mxic_nand.c
> 
> diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
> index e604625..e0329cc 100644
> --- a/drivers/mtd/nand/raw/Kconfig
> +++ b/drivers/mtd/nand/raw/Kconfig
> @@ -522,6 +522,12 @@ config MTD_NAND_QCOM
>  	  Enables support for NAND flash chips on SoCs containing the EBI2 NAND
>  	  controller. This controller is found on IPQ806x SoC.
>  
> +config MTD_NAND_MXIC
> +	tristate "Macronix MX25F0A NAND controller"
> +	depends on HAS_IOMEM
> +	help
> +	  This selects the Macronix MX25F0A NAND controller driver.
> +
>  config MTD_NAND_MTK
>  	tristate "Support for NAND controller on MTK SoCs"
>  	depends on ARCH_MEDIATEK || COMPILE_TEST
> diff --git a/drivers/mtd/nand/raw/Makefile b/drivers/mtd/nand/raw/Makefile
> index 5a5a72f..c8a6790 100644
> --- a/drivers/mtd/nand/raw/Makefile
> +++ b/drivers/mtd/nand/raw/Makefile
> @@ -54,6 +54,7 @@ obj-$(CONFIG_MTD_NAND_SUNXI)		+= sunxi_nand.o
>  obj-$(CONFIG_MTD_NAND_HISI504)	        += hisi504_nand.o
>  obj-$(CONFIG_MTD_NAND_BRCMNAND)		+= brcmnand/
>  obj-$(CONFIG_MTD_NAND_QCOM)		+= qcom_nandc.o
> +obj-$(CONFIG_MTD_NAND_MXIC)		+= mxic_nand.o
>  obj-$(CONFIG_MTD_NAND_MTK)		+= mtk_ecc.o mtk_nand.o
>  obj-$(CONFIG_MTD_NAND_TEGRA)		+= tegra_nand.o
>  obj-$(CONFIG_MTD_NAND_STM32_FMC2)	+= stm32_fmc2_nand.o
> diff --git a/drivers/mtd/nand/raw/mxic_nand.c b/drivers/mtd/nand/raw/mxic_nand.c
> new file mode 100644
> index 0000000..689fae2
> --- /dev/null
> +++ b/drivers/mtd/nand/raw/mxic_nand.c
> @@ -0,0 +1,294 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// Copyright (C) 2019 Macronix International Co., Ltd.
> +//
> +// Authors:
> +//	Mason Yang <masonccyang@mxic.com.tw>
> +//	zhengxunli <zhengxunli@mxic.com.tw>

This is not a valid name.

Also if he appears here I suppose he should be credited in the
module_authors() macro too.

> +//

As a personal taste, I prefer when the header uses /* */ and only the
SPDX tag uses //.

> +
> +#include <linux/mfd/mxic-mx25f0a.h>
> +#include <linux/mtd/rawnand.h>
> +#include <linux/mtd/nand_ecc.h>
> +
> +#include "internals.h"
> +
> +struct mxic_nand_ctlr {
> +	struct mx25f0a_mfd *mfd;
> +	struct nand_controller base;
> +	struct nand_chip nand;

Even if this controller only supports one CS (and then, one chip),
please have a clear separation between the NAND controller and the NAND
chip by having one structure for the NAND chips and one structure for
the NAND controller.

> +};
> +
> +static void mxic_host_init(struct mxic_nand_ctlr *mxic)

Please choose a constant prefix for all functions, right now there is:
mxic_
mxic_nand_
mx25f0a_nand_

I think mxic_nand_ or mx25f0a_nand_ is wise.

> +{
> +	writel(DATA_STROB_EDO_EN, mxic->mfd->regs + DATA_STROB);
> +	writel(INT_STS_ALL, mxic->mfd->regs + INT_STS_EN);
> +	writel(0x0, mxic->mfd->regs + ONFI_DIN_CNT(0));
> +	writel(HC_CFG_NIO(8) | HC_CFG_SLV_ACT(0) | HC_CFG_IDLE_SIO_LVL(1) |
> +	       HC_CFG_TYPE(1, HC_CFG_TYPE_RAW_NAND) | HC_CFG_MAN_CS_EN,
> +	       mxic->mfd->regs + HC_CFG);
> +	writel(0x0, mxic->mfd->regs + HC_EN);
> +}
> +
> +static int  mxic_nand_wait_ready(struct nand_chip *chip)
> +{
> +	struct mxic_nand_ctlr *mxic = nand_get_controller_data(chip);
> +	u32 sts;
> +
> +	return readl_poll_timeout(mxic->mfd->regs + INT_STS, sts,
> +				  sts & INT_RDY_PIN, 0, USEC_PER_SEC);
> +}
> +
> +static void mxic_nand_select_chip(struct nand_chip *chip, int chipnr)

_select_target() is preferred now

> +{
> +	struct mxic_nand_ctlr *mxic = nand_get_controller_data(chip);
> +
> +	switch (chipnr) {
> +	case 0:
> +	case 1:
> +		writel(HC_EN_BIT, mxic->mfd->regs + HC_EN);
> +		writel(HC_CFG_MAN_CS_ASSERT | readl(mxic->mfd->regs + HC_CFG),
> +		       mxic->mfd->regs + HC_CFG);

In both case I would prefer a:

        reg = readl(...);
        reg &= ~xxx;
	reg |= yyy;
	writel(reg, ...);

Much easier to read.

> +		break;
> +
> +	case -1:
> +		writel(~HC_CFG_MAN_CS_ASSERT & readl(mxic->mfd->regs + HC_CFG),
> +		       mxic->mfd->regs + HC_CFG);
> +		writel(0, mxic->mfd->regs + HC_EN);
> +		break;
> +
> +	default:

Error?

> +		break;
> +	}
> +}
> +
> +static int mxic_nand_data_xfer(struct mxic_nand_ctlr *mxic, const void *txbuf,
> +			       void *rxbuf, unsigned int len)
> +{

There is not so much code shared, why not separating this function for
rx and tx cases?

> +	unsigned int pos = 0;
> +
> +	while (pos < len) {
> +		unsigned int nbytes = len - pos;
> +		u32 data = 0xffffffff;
> +		u32 sts;
> +		int ret;
> +
> +		if (nbytes > 4)
> +			nbytes = 4;
> +
> +		if (txbuf)
> +			memcpy(&data, txbuf + pos, nbytes);
> +
> +		ret = readl_poll_timeout(mxic->mfd->regs + INT_STS, sts,
> +					 sts & INT_TX_EMPTY, 0, USEC_PER_SEC);

Using USEC_PER_SEC for a delay is weird

> +		if (ret)
> +			return ret;
> +
> +		writel(data, mxic->mfd->regs + TXD(nbytes % 4));
> +
> +		if (rxbuf) {
> +			ret = readl_poll_timeout(mxic->mfd->regs + INT_STS, sts,
> +						 sts & INT_TX_EMPTY, 0,
> +						 USEC_PER_SEC);
> +			if (ret)
> +				return ret;
> +
> +			ret = readl_poll_timeout(mxic->mfd->regs + INT_STS, sts,
> +						 sts & INT_RX_NOT_EMPTY, 0,
> +						 USEC_PER_SEC);
> +			if (ret)
> +				return ret;
> +
> +			data = readl(mxic->mfd->regs + RXD);
> +			data >>= (8 * (4 - nbytes));

What is this? Are you trying to handle some endianness issue?

> +			memcpy(rxbuf + pos, &data, nbytes);
> +			WARN_ON(readl(mxic->mfd->regs + INT_STS) &
> +				INT_RX_NOT_EMPTY);
> +		} else {
> +			readl(mxic->mfd->regs + RXD);
> +		}
> +		WARN_ON(readl(mxic->mfd->regs + INT_STS) & INT_RX_NOT_EMPTY);
> +
> +		pos += nbytes;
> +	}
> +
> +	return 0;
> +}
> +
> +static int mxic_nand_exec_op(struct nand_chip *chip,
> +			     const struct nand_operation *op, bool check_only)
> +{
> +	struct mxic_nand_ctlr *mxic = nand_get_controller_data(chip);
> +	const struct nand_op_instr *instr = NULL;
> +	int i, len = 0, ret = 0;
> +	unsigned int op_id;
> +	unsigned char cmdcnt = 0, addr_cnt = 0, cmd_addr[8] = {0};
> +
> +	for (op_id = 0; op_id < op->ninstrs; op_id++) {
> +		instr = &op->instrs[op_id];
> +
> +		switch (instr->type) {
> +		case NAND_OP_CMD_INSTR:
> +			cmd_addr[len++] = instr->ctx.cmd.opcode;
> +			cmdcnt++;
> +			break;
> +
> +		case NAND_OP_ADDR_INSTR:
> +			for (i = 0; i < instr->ctx.addr.naddrs; i++)
> +				cmd_addr[len++] = instr->ctx.addr.addrs[i];
> +			addr_cnt = i;
> +			break;
> +
> +		case NAND_OP_DATA_IN_INSTR:
> +			break;
> +
> +		case NAND_OP_DATA_OUT_INSTR:
> +			writel(instr->ctx.data.len,
> +			       mxic->mfd->regs + ONFI_DIN_CNT(0));
> +			break;
> +
> +		case NAND_OP_WAITRDY_INSTR:
> +			break;
> +		}
> +	}
> +
> +	if (op_id == 5 && instr->type == NAND_OP_WAITRDY_INSTR) {
> +		/*
> +		 * In case cmd-addr-data-cmd-wait in a sequence,
> +		 * separate the 2'nd command, i.e,. nand_prog_page_op()
> +		 */

I think this is the kind of limitation that could be described very
easily with a nand_op_parser structure and used by
nand_op_parser_exec_op() (see marvell_nand.c).

> +		writel(OP_CMD_BUSW(OP_BUSW_8) | OP_ADDR_BUSW(OP_BUSW_8) |
> +		       OP_DATA_BUSW(OP_BUSW_8) | OP_DUMMY_CYC(0x3F) |
> +		       OP_ADDR_BYTES(addr_cnt) |
> +		       OP_CMD_BYTES(1), mxic->mfd->regs + SS_CTRL(0));
> +		writel(0, mxic->mfd->regs + HC_EN);
> +		writel(HC_EN_BIT, mxic->mfd->regs + HC_EN);
> +
> +		mxic_nand_data_xfer(mxic, cmd_addr, NULL, len - 1);
> +
> +		mxic_nand_data_xfer(mxic, instr->ctx.data.buf.out, NULL,
> +				    instr->ctx.data.len);
> +
> +		writel(0, mxic->mfd->regs + HC_EN);
> +		writel(HC_EN_BIT, mxic->mfd->regs + HC_EN);
> +		mxic_nand_data_xfer(mxic, &cmd_addr[--len], NULL, 1);
> +		ret = mxic_nand_wait_ready(chip);
> +		if (ret)
> +			goto err_out;
> +		return ret;
> +	}
> +
> +	if (len) {
> +		writel(OP_CMD_BUSW(OP_BUSW_8) | OP_ADDR_BUSW(OP_BUSW_8) |
> +		       OP_DATA_BUSW(OP_BUSW_8) | OP_DUMMY_CYC(0x3F) |
> +		       OP_ADDR_BYTES(addr_cnt) |
> +		       OP_CMD_BYTES(cmdcnt > 0 ? cmdcnt : 0),
> +		       mxic->mfd->regs + SS_CTRL(0));
> +		writel(0, mxic->mfd->regs + HC_EN);
> +		writel(HC_EN_BIT, mxic->mfd->regs + HC_EN);
> +
> +		mxic_nand_data_xfer(mxic, cmd_addr, NULL, len);
> +	}
> +
> +	for (op_id = 0; op_id < op->ninstrs; op_id++) {
> +		instr = &op->instrs[op_id];
> +
> +		switch (instr->type) {
> +		case NAND_OP_CMD_INSTR:
> +		case NAND_OP_ADDR_INSTR:
> +			break;
> +
> +		case NAND_OP_DATA_IN_INSTR:
> +			writel(0x0, mxic->mfd->regs + ONFI_DIN_CNT(0));
> +			writel(readl(mxic->mfd->regs + SS_CTRL(0)) | OP_READ,
> +			       mxic->mfd->regs + SS_CTRL(0));
> +			mxic_nand_data_xfer(mxic, NULL, instr->ctx.data.buf.in,
> +					    instr->ctx.data.len);
> +			break;
> +
> +		case NAND_OP_DATA_OUT_INSTR:
> +			mxic_nand_data_xfer(mxic, instr->ctx.data.buf.out, NULL,
> +					    instr->ctx.data.len);
> +			break;
> +
> +		case NAND_OP_WAITRDY_INSTR:
> +			ret = mxic_nand_wait_ready(chip);
> +			if (ret)
> +				goto err_out;
> +			break;
> +		}
> +	}
> +
> +err_out:
> +	return ret;

Ditto, please return directly if there is nothing in the error path.

> +}
> +
> +static const struct nand_controller_ops mxic_nand_controller_ops = {
> +	.exec_op = mxic_nand_exec_op,
> +};
> +
> +static int mx25f0a_nand_probe(struct platform_device *pdev)
> +{
> +	struct mtd_info *mtd;
> +	struct mx25f0a_mfd *mfd = dev_get_drvdata(pdev->dev.parent);
> +	struct mxic_nand_ctlr *mxic;
> +	struct nand_chip *nand_chip;
> +	int err;
> +
> +	mxic = devm_kzalloc(&pdev->dev, sizeof(struct mxic_nand_ctlr),
> +			    GFP_KERNEL);

mxic for a NAND controller structure is probably not a name meaningful
enough.

> +	if (!mxic)
> +		return -ENOMEM;
> +
> +	nand_chip = &mxic->nand;
> +	mtd = nand_to_mtd(nand_chip);
> +	mtd->dev.parent = pdev->dev.parent;
> +	nand_chip->ecc.priv = NULL;
> +	nand_set_flash_node(nand_chip, pdev->dev.parent->of_node);
> +	nand_chip->priv = mxic;
> +
> +	mxic->mfd = mfd;
> +
> +	nand_chip->legacy.select_chip = mxic_nand_select_chip;

Please don't implement legacy interfaces. You can check in
marvell_nand.c how this is handled now:

b25251414f6e mtd: rawnand: marvell: Stop implementing ->select_chip()

> +	mxic->base.ops = &mxic_nand_controller_ops;
> +	nand_controller_init(&mxic->base);
> +	nand_chip->controller = &mxic->base;
> +
> +	mxic_host_init(mxic);
> +
> +	err = nand_scan(nand_chip, 1);
> +	if (err)
> +		goto fail;

You can return directly as there is nothing to clean in the error path.

> +
> +	err = mtd_device_register(mtd, NULL, 0);
> +	if (err)
> +		goto fail;

Ditto

> +
> +	platform_set_drvdata(pdev, mxic);
> +
> +	return 0;
> +fail:
> +	return err;
> +}
> +
> +static int mx25f0a_nand_remove(struct platform_device *pdev)
> +{
> +	struct mxic_nand_ctlr *mxic = platform_get_drvdata(pdev);
> +
> +	nand_release(&mxic->nand);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver mx25f0a_nand_driver = {
> +	.probe		= mx25f0a_nand_probe,

Please don't align '=' on tabs.

> +	.remove		= mx25f0a_nand_remove,
> +	.driver		= {
> +		.name	= "mxic-nand-ctlr",
> +	},
> +};
> +module_platform_driver(mx25f0a_nand_driver);

mx25f0a_nand_controller_driver would be better

> +
> +MODULE_AUTHOR("Mason Yang <masonccyang@mxic.com.tw>");
> +MODULE_DESCRIPTION("MX25F0A RAW NAND controller driver");
> +MODULE_LICENSE("GPL v2");

Thanks,
Miquèl

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

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

* Re: [PATCH v3 4/4] dt-bindings: mfd: Document Macronix MX25F0A controller bindings
  2019-04-15  9:23 ` [PATCH v3 4/4] dt-bindings: mfd: Document Macronix MX25F0A controller bindings Mason Yang
  2019-04-26 22:41   ` Rob Herring
@ 2019-05-12 13:23   ` Miquel Raynal
  2019-05-15  7:36     ` masonccyang
  1 sibling, 1 reply; 21+ messages in thread
From: Miquel Raynal @ 2019-05-12 13:23 UTC (permalink / raw)
  To: Mason Yang
  Cc: mark.rutland, devicetree, christophe.kerello, juliensu,
	bbrezillon, marcel.ziswiler, lee.jones, linux-kernel, robh+dt,
	linux-spi, marek.vasut, paul.burton, broonie, geert, stefan,
	linux-mtd, richard, liang.yang, computersforpeace, dwmw2,
	zhengxunli

Hi Mason,

Mason Yang <masonccyang@mxic.com.tw> wrote on Mon, 15 Apr 2019 17:23:54
+0800:

> Document the bindings used by the Macronix MX25F0A MFD controller.
> 
> Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
> ---
>  .../devicetree/bindings/mfd/mxic-mx25f0a.txt       | 51 ++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/mxic-mx25f0a.txt
> 
> diff --git a/Documentation/devicetree/bindings/mfd/mxic-mx25f0a.txt b/Documentation/devicetree/bindings/mfd/mxic-mx25f0a.txt
> new file mode 100644
> index 0000000..7f3e0f8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/mxic-mx25f0a.txt
> @@ -0,0 +1,51 @@
> +Macronix MX25F0A MultiFunction Device Tree Bindings
> +----------------------------------------------------
> +
> +MX25F0A is a MultiFunction Device with SPI and raw NAND, which
> +supports either spi host controller or raw nand controller.

Acronyms in plain English should be in upper case.

> +
> +Required properties:
> +- compatible: should be "mxic,mx25f0a"
> +- #address-cells: should be 1
> +- #size-cells: should be 0
> +- reg: should contain 2 entries, one for the registers and one for the direct
> +       mapping area in SPI mode.
> +- reg-names: should contain "regs" and "dirmap"
> +- interrupts: interrupt line connected to this MFD controller
> +- SPI controller driver:
> +		- clock-names: should contain "ps_clk", "send_clk" and
> +			       "send_dly_clk"
> +		- clocks: should contain 3 entries for the "ps_clk", "send_clk"
> +			  and "send_dly_clk" clocks
> +
> +- Raw nand controller driver.
> +		- nand-ecc-mode = "soft";
> +		- nand-ecc-algo = "bch";
> +
> +Example:
> +
> +	mxic: mx25f0a@43c30000 {
> +		compatible = "mxic,mx25f0a";
> +		reg = <0x43c30000 0x10000>, <0xa0000000 0x4000000>;
> +		reg-names = "regs", "dirmap";
> +
> +		/* spi */
> +		clocks = <&clkwizard 0>, <&clkwizard 1>, <&clkc 15>;
> +		clock-names = "send_clk", "send_dly_clk", "ps_clk";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		flash@0 {
> +			compatible = "jedec,spi-nor";
> +			reg = <0>;
> +			spi-max-frequency = <25000000>;
> +			spi-tx-bus-width = <4>;
> +			spi-rx-bus-width = <4>;
> +		};
> +
> +		/* nand */
> +		nand-ecc-mode = "soft";
> +		nand-ecc-algo = "bch";
> +		nand-ecc-step-size = <512>;
> +		nand-ecc-strength = <8>;

Any reason to enforce 512B/8b correction? Why not letting the core
choose for you depending on the NAND chip's requirements?


Anyway, I think you can have only one or the other (NAND or SPI), not
both, and you probably should have a compatible or a property to tell
the kernel which one you are using, right?


Thanks,
Miquèl

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

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

* Re: [PATCH v3 1/4]  mfd: Add Macronix MX25F0A MFD controller driver
  2019-05-12 13:08   ` Miquel Raynal
@ 2019-05-15  6:46     ` masonccyang
  0 siblings, 0 replies; 21+ messages in thread
From: masonccyang @ 2019-05-15  6:46 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: mark.rutland, devicetree, christophe.kerello, bbrezillon,
	juliensu, lee.jones, linux-kernel, robh+dt, linux-spi,
	marcel.ziswiler, paul.burton, broonie, geert, stefan, linux-mtd,
	richard, liang.yang, computersforpeace, dwmw2, marek.vasut,
	zhengxunli


Hi Miquel,

 
> > Add a driver for Macronix MX25F0A multifunction device controller.
> > 
> > Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
> > ---
> >  drivers/mfd/Kconfig              |   9 ++
> >  drivers/mfd/Makefile             |   1 +
> >  drivers/mfd/mxic-mx25f0a.c       |  84 +++++++++++++++++++
> >  include/linux/mfd/mxic-mx25f0a.h | 175 
+++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 269 insertions(+)
> >  create mode 100644 drivers/mfd/mxic-mx25f0a.c
> >  create mode 100644 include/linux/mfd/mxic-mx25f0a.h
> > 
> > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> > index 26ad646..7e99e93 100644
> > --- a/drivers/mfd/Kconfig
> > +++ b/drivers/mfd/Kconfig
> > @@ -823,6 +823,15 @@ config MFD_MAX8998
> >       additional drivers must be enabled in order to use the 
functionality
> >       of the device.
> > 
> > +config MFD_MXIC_MX25F0A
> > +   tristate "Macronix mx25f0a multifunction device support"
> > +   select MFD_CORE
> > +   help
> > +     This supports for Macronix mx25f0a multifunction device 
controller
> > +     for raw nand or spi. You have to select individual components 
like
> 
> Please use upper case for acronyms in plain English: NAND, SPI

okay, will fix.

And I also would like to remove "mx25f0a" char.

patch to:
------------------------------------------------------------------------
config MFD_MXIC_FMC
   tristate "Macronix Flash Memory Controller support"
   select MFD_CORE
   help
     This supports Macronix Flash Memory Controller for raw NAND and SPI.
     You have to select individual components like raw NAND controller
     or SPI host controller under the corresponding  menus.
-------------------------------------------------------------------------

thanks for your review.

best regards,
Mason




CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information 
and/or personal data, which is protected by applicable laws. Please be 
reminded that duplication, disclosure, distribution, or use of this e-mail 
(and/or its attachments) or any part thereof is prohibited. If you receive 
this e-mail in error, please notify us immediately and delete this mail as 
well as its attachment(s) from your system. In addition, please be 
informed that collection, processing, and/or use of personal data is 
prohibited unless expressly permitted by personal data protection laws. 
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================



============================================================================

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================


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

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

* Re: [PATCH v3 4/4] dt-bindings: mfd: Document Macronix MX25F0A controller bindings
  2019-05-12 13:23   ` Miquel Raynal
@ 2019-05-15  7:36     ` masonccyang
  0 siblings, 0 replies; 21+ messages in thread
From: masonccyang @ 2019-05-15  7:36 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: mark.rutland, devicetree, christophe.kerello, bbrezillon,
	juliensu, lee.jones, linux-kernel, robh+dt, linux-spi,
	marcel.ziswiler, paul.burton, broonie, geert, stefan, linux-mtd,
	richard, liang.yang, computersforpeace, dwmw2, marek.vasut,
	zhengxunli


Hi Miquel,


> > +Macronix MX25F0A MultiFunction Device Tree Bindings
> > +----------------------------------------------------
> > +
> > +MX25F0A is a MultiFunction Device with SPI and raw NAND, which
> > +supports either spi host controller or raw nand controller.
> 
> Acronyms in plain English should be in upper case.

okay, will fix.

> > +Example:
> > +
> > +   mxic: mx25f0a@43c30000 {
> > +      compatible = "mxic,mx25f0a";
> > +      reg = <0x43c30000 0x10000>, <0xa0000000 0x4000000>;
> > +      reg-names = "regs", "dirmap";
> > +
> > +      /* spi */
> > +      clocks = <&clkwizard 0>, <&clkwizard 1>, <&clkc 15>;
> > +      clock-names = "send_clk", "send_dly_clk", "ps_clk";
> > +      #address-cells = <1>;
> > +      #size-cells = <0>;
> > +
> > +      flash@0 {
> > +         compatible = "jedec,spi-nor";
> > +         reg = <0>;
> > +         spi-max-frequency = <25000000>;
> > +         spi-tx-bus-width = <4>;
> > +         spi-rx-bus-width = <4>;
> > +      };
> > +
> > +      /* nand */
> > +      nand-ecc-mode = "soft";
> > +      nand-ecc-algo = "bch";
> > +      nand-ecc-step-size = <512>;
> > +      nand-ecc-strength = <8>;
> 
> Any reason to enforce 512B/8b correction? Why not letting the core
> choose for you depending on the NAND chip's requirements?
> 

I thought here is just a raw NAND DTS example. 
Will remove it.

> 
> Anyway, I think you can have only one or the other (NAND or SPI), not
> both, and you probably should have a compatible or a property to tell
> the kernel which one you are using, right?
> 

yes, you are right.

New DTS is bellow.
-------------------------------------------------------------------------->

Macronix Flash Memory Controller Device Tree Bindings
-----------------------------------------------------

Macronix Flash Memory Controller supports serial and raw Flash, including
NOR and NAND Flash for high throughput and low pin count applications.
It's a MultiFunction Device which supports either SPI host controller or
raw NAND controller.

Required properties:
- compatible: should be "mxic,mfd"
- reg: should contain 2 entries, one for the registers and one for the 
direct
       mapping area in SPI mode.
- reg-names: should contain "regs" and "dirmap"
- interrupts: interrupt line connected to this controller
- SPI:
        - #address-cells: should be 1
        - #size-cells: should be 0
        - clock-names: should contain "ps_clk", "send_clk" and
                       "send_dly_clk"
        - clocks: should contain 3 entries for the "ps_clk", "send_clk"
                  and "send_dly_clk" clocks
- Raw NAND:
        - nand-ecc-mode = "soft";
        - nand-ecc-algo = "bch";

Example:
- SPI mode:

        mxic: mxic-mfd@43c30000 {
                compatible = "mxic,mfd";
                reg = <0x43c30000 0x10000>, <0xa0000000 0x4000000>;
                reg-names = "regs", "dirmap";
                clocks = <&clkwizard 0>, <&clkwizard 1>, <&clkc 15>;
                clock-names = "send_clk", "send_dly_clk", "ps_clk";
                #address-cells = <1>;
                #size-cells = <0>;

                flash@0 {
                        compatible = "jedec,spi-nor";
                        reg = <0>;
                        spi-max-frequency = <25000000>;
                        spi-tx-bus-width = <4>;
                        spi-rx-bus-width = <4>;
                };
        };
 
- Raw NAND mode:

        mxic: mxic-mfd@43c30000 {
                compatible = "mxic,mfd";
                reg = <0x43c30000 0x10000>, <0xa0000000 0x4000000>;
                reg-names = "regs", "dirmap";

                nand-ecc-mode = "soft";
                nand-ecc-algo = "bch";
        };  

---------------------------------------------------------------------< 

thanks for your review.

best regards,
Mason



CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information 
and/or personal data, which is protected by applicable laws. Please be 
reminded that duplication, disclosure, distribution, or use of this e-mail 
(and/or its attachments) or any part thereof is prohibited. If you receive 
this e-mail in error, please notify us immediately and delete this mail as 
well as its attachment(s) from your system. In addition, please be 
informed that collection, processing, and/or use of personal data is 
prohibited unless expressly permitted by personal data protection laws. 
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================



============================================================================

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================


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

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

* Re: [PATCH v3 2/4] mtd: rawnand: Add Macronix MX25F0A NAND controller
  2019-05-12 13:18   ` Miquel Raynal
@ 2019-05-15  8:48     ` masonccyang
  2019-05-15 12:08       ` Miquel Raynal
       [not found]     ` <OF8A566F14.A2F0F576-ON482583FB.002E7E32-482583FB.003068B1@LocalDomain>
  2019-05-17  9:30     ` masonccyang
  2 siblings, 1 reply; 21+ messages in thread
From: masonccyang @ 2019-05-15  8:48 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: mark.rutland, devicetree, christophe.kerello, bbrezillon,
	juliensu, lee.jones, linux-kernel, robh+dt, linux-spi,
	marcel.ziswiler, paul.burton, broonie, geert, stefan, linux-mtd,
	richard, liang.yang, computersforpeace, dwmw2, marek.vasut,
	zhengxunli


Hi Miquel,

> > +// SPDX-License-Identifier: GPL-2.0
> > +//
> > +// Copyright (C) 2019 Macronix International Co., Ltd.
> > +//
> > +// Authors:
> > +//   Mason Yang <masonccyang@mxic.com.tw>
> > +//   zhengxunli <zhengxunli@mxic.com.tw>
> 
> This is not a valid name.
> 
> Also if he appears here I suppose he should be credited in the
> module_authors() macro too.

I think Li should maintain this NAND driver later, 
anyway, I may remove him.

> 
> > +//
> 
> As a personal taste, I prefer when the header uses /* */ and only the
> SPDX tag uses //.

okay, only SPDX tag use //

> 
> > +
> > +#include <linux/mfd/mxic-mx25f0a.h>
> > +#include <linux/mtd/rawnand.h>
> > +#include <linux/mtd/nand_ecc.h>
> > +
> > +#include "internals.h"
> > +
> > +struct mxic_nand_ctlr {
> > +   struct mx25f0a_mfd *mfd;
> > +   struct nand_controller base;
> > +   struct nand_chip nand;
> 
> Even if this controller only supports one CS (and then, one chip),
> please have a clear separation between the NAND controller and the NAND
> chip by having one structure for the NAND chips and one structure for
> the NAND controller.

okay, will patch it.

> 
> > +};
> > +
> > +static void mxic_host_init(struct mxic_nand_ctlr *mxic)
> 
> Please choose a constant prefix for all functions, right now there is:
> mxic_
> mxic_nand_
> mx25f0a_nand_
> 
> I think mxic_nand_ or mx25f0a_nand_ is wise.

okay, will use mxic_nand_ as prefix. 

> 
> > +{
> > +   writel(DATA_STROB_EDO_EN, mxic->mfd->regs + DATA_STROB);
> > +   writel(INT_STS_ALL, mxic->mfd->regs + INT_STS_EN);
> > +   writel(0x0, mxic->mfd->regs + ONFI_DIN_CNT(0));
> > +   writel(HC_CFG_NIO(8) | HC_CFG_SLV_ACT(0) | HC_CFG_IDLE_SIO_LVL(1) 
|
> > +          HC_CFG_TYPE(1, HC_CFG_TYPE_RAW_NAND) | HC_CFG_MAN_CS_EN,
> > +          mxic->mfd->regs + HC_CFG);
> > +   writel(0x0, mxic->mfd->regs + HC_EN);
> > +}
> > +
> > +static int  mxic_nand_wait_ready(struct nand_chip *chip)
> > +{
> > +   struct mxic_nand_ctlr *mxic = nand_get_controller_data(chip);
> > +   u32 sts;
> > +
> > +   return readl_poll_timeout(mxic->mfd->regs + INT_STS, sts,
> > +              sts & INT_RDY_PIN, 0, USEC_PER_SEC);
> > +}
> > +
> > +static void mxic_nand_select_chip(struct nand_chip *chip, int chipnr)
> 
> _select_target() is preferred now
> 
> > +{
> > +   struct mxic_nand_ctlr *mxic = nand_get_controller_data(chip);
> > +
> > +   switch (chipnr) {
> > +   case 0:
> > +   case 1:
> > +      writel(HC_EN_BIT, mxic->mfd->regs + HC_EN);
> > +      writel(HC_CFG_MAN_CS_ASSERT | readl(mxic->mfd->regs + HC_CFG),
> > +             mxic->mfd->regs + HC_CFG);
> 
> In both case I would prefer a:
> 
>         reg = readl(...);
>         reg &= ~xxx;
>    reg |= yyy;
>    writel(reg, ...);
> 
> Much easier to read.
> 
> > +      break;
> > +
> > +   case -1:
> > +      writel(~HC_CFG_MAN_CS_ASSERT & readl(mxic->mfd->regs + HC_CFG),
> > +             mxic->mfd->regs + HC_CFG);
> > +      writel(0, mxic->mfd->regs + HC_EN);
> > +      break;
> > +
> > +   default:
> 
> Error?
> 
> > +      break;
> > +   }
> > +}
> > +
> > +static int mxic_nand_data_xfer(struct mxic_nand_ctlr *mxic, const 
void *txbuf,
> > +                void *rxbuf, unsigned int len)
> > +{
> 
> There is not so much code shared, why not separating this function for
> rx and tx cases?
> 
> > +   unsigned int pos = 0;
> > +
> > +   while (pos < len) {
> > +      unsigned int nbytes = len - pos;
> > +      u32 data = 0xffffffff;
> > +      u32 sts;
> > +      int ret;
> > +
> > +      if (nbytes > 4)
> > +         nbytes = 4;
> > +
> > +      if (txbuf)
> > +         memcpy(&data, txbuf + pos, nbytes);
> > +
> > +      ret = readl_poll_timeout(mxic->mfd->regs + INT_STS, sts,
> > +                sts & INT_TX_EMPTY, 0, USEC_PER_SEC);
> 
> Using USEC_PER_SEC for a delay is weird
> 
> > +      if (ret)
> > +         return ret;
> > +
> > +      writel(data, mxic->mfd->regs + TXD(nbytes % 4));
> > +
> > +      if (rxbuf) {
> > +         ret = readl_poll_timeout(mxic->mfd->regs + INT_STS, sts,
> > +                   sts & INT_TX_EMPTY, 0,
> > +                   USEC_PER_SEC);
> > +         if (ret)
> > +            return ret;
> > +
> > +         ret = readl_poll_timeout(mxic->mfd->regs + INT_STS, sts,
> > +                   sts & INT_RX_NOT_EMPTY, 0,
> > +                   USEC_PER_SEC);
> > +         if (ret)
> > +            return ret;
> > +
> > +         data = readl(mxic->mfd->regs + RXD);
> > +         data >>= (8 * (4 - nbytes));
> 
> What is this? Are you trying to handle some endianness issue?

yes,

> 
> > +         memcpy(rxbuf + pos, &data, nbytes);
> > +         WARN_ON(readl(mxic->mfd->regs + INT_STS) &
> > +            INT_RX_NOT_EMPTY);
> > +      } else {
> > +         readl(mxic->mfd->regs + RXD);
> > +      }
> > +      WARN_ON(readl(mxic->mfd->regs + INT_STS) & INT_RX_NOT_EMPTY);
> > +
> > +      pos += nbytes;
> > +   }
> > +
> > +   return 0;
> > +}






> > +
> > +static int mxic_nand_exec_op(struct nand_chip *chip,
> > +              const struct nand_operation *op, bool check_only)
> > +{
> > +   struct mxic_nand_ctlr *mxic = nand_get_controller_data(chip);
> > +   const struct nand_op_instr *instr = NULL;
> > +   int i, len = 0, ret = 0;
> > +   unsigned int op_id;
> > +   unsigned char cmdcnt = 0, addr_cnt = 0, cmd_addr[8] = {0};
> > +
> > +   for (op_id = 0; op_id < op->ninstrs; op_id++) {
> > +      instr = &op->instrs[op_id];
> > +
> > +      switch (instr->type) {
> > +      case NAND_OP_CMD_INSTR:
> > +         cmd_addr[len++] = instr->ctx.cmd.opcode;
> > +         cmdcnt++;
> > +         break;
> > +
> > +      case NAND_OP_ADDR_INSTR:
> > +         for (i = 0; i < instr->ctx.addr.naddrs; i++)
> > +            cmd_addr[len++] = instr->ctx.addr.addrs[i];
> > +         addr_cnt = i;
> > +         break;
> > +
> > +      case NAND_OP_DATA_IN_INSTR:
> > +         break;
> > +
> > +      case NAND_OP_DATA_OUT_INSTR:
> > +         writel(instr->ctx.data.len,
> > +                mxic->mfd->regs + ONFI_DIN_CNT(0));
> > +         break;
> > +
> > +      case NAND_OP_WAITRDY_INSTR:
> > +         break;
> > +      }
> > +   }
> > +
> > +   if (op_id == 5 && instr->type == NAND_OP_WAITRDY_INSTR) {
> > +      /*
> > +       * In case cmd-addr-data-cmd-wait in a sequence,
> > +       * separate the 2'nd command, i.e,. nand_prog_page_op()
> > +       */
> 
> I think this is the kind of limitation that could be described very
> easily with a nand_op_parser structure and used by
> nand_op_parser_exec_op() (see marvell_nand.c).

okay, thanks for your information.
Will check and patch it.

> 
> > +      writel(OP_CMD_BUSW(OP_BUSW_8) | OP_ADDR_BUSW(OP_BUSW_8) |
> > +             OP_DATA_BUSW(OP_BUSW_8) | OP_DUMMY_CYC(0x3F) |
> > +             OP_ADDR_BYTES(addr_cnt) |
> > +             OP_CMD_BYTES(1), mxic->mfd->regs + SS_CTRL(0));
> > +      writel(0, mxic->mfd->regs + HC_EN);
> > +      writel(HC_EN_BIT, mxic->mfd->regs + HC_EN);
> > +
> > +      mxic_nand_data_xfer(mxic, cmd_addr, NULL, len - 1);
> > +
> > +      mxic_nand_data_xfer(mxic, instr->ctx.data.buf.out, NULL,
> > +                instr->ctx.data.len);
> > +
> > +      writel(0, mxic->mfd->regs + HC_EN);
> > +      writel(HC_EN_BIT, mxic->mfd->regs + HC_EN);
> > +      mxic_nand_data_xfer(mxic, &cmd_addr[--len], NULL, 1);
> > +      ret = mxic_nand_wait_ready(chip);
> > +      if (ret)
> > +         goto err_out;
> > +      return ret;
> > +   }
> > +
> > +   if (len) {
> > +      writel(OP_CMD_BUSW(OP_BUSW_8) | OP_ADDR_BUSW(OP_BUSW_8) |
> > +             OP_DATA_BUSW(OP_BUSW_8) | OP_DUMMY_CYC(0x3F) |
> > +             OP_ADDR_BYTES(addr_cnt) |
> > +             OP_CMD_BYTES(cmdcnt > 0 ? cmdcnt : 0),
> > +             mxic->mfd->regs + SS_CTRL(0));
> > +      writel(0, mxic->mfd->regs + HC_EN);
> > +      writel(HC_EN_BIT, mxic->mfd->regs + HC_EN);
> > +
> > +      mxic_nand_data_xfer(mxic, cmd_addr, NULL, len);
> > +   }
> > +
> > +   for (op_id = 0; op_id < op->ninstrs; op_id++) {
> > +      instr = &op->instrs[op_id];
> > +
> > +      switch (instr->type) {
> > +      case NAND_OP_CMD_INSTR:
> > +      case NAND_OP_ADDR_INSTR:
> > +         break;
> > +
> > +      case NAND_OP_DATA_IN_INSTR:
> > +         writel(0x0, mxic->mfd->regs + ONFI_DIN_CNT(0));
> > +         writel(readl(mxic->mfd->regs + SS_CTRL(0)) | OP_READ,
> > +                mxic->mfd->regs + SS_CTRL(0));
> > +         mxic_nand_data_xfer(mxic, NULL, instr->ctx.data.buf.in,
> > +                   instr->ctx.data.len);
> > +         break;
> > +
> > +      case NAND_OP_DATA_OUT_INSTR:
> > +         mxic_nand_data_xfer(mxic, instr->ctx.data.buf.out, NULL,
> > +                   instr->ctx.data.len);
> > +         break;
> > +
> > +      case NAND_OP_WAITRDY_INSTR:
> > +         ret = mxic_nand_wait_ready(chip);
> > +         if (ret)
> > +            goto err_out;
> > +         break;
> > +      }
> > +   }
> > +
> > +err_out:
> > +   return ret;
> 
> Ditto, please return directly if there is nothing in the error path.

okay

> 
> > +}
> > +
> > +static const struct nand_controller_ops mxic_nand_controller_ops = {
> > +   .exec_op = mxic_nand_exec_op,
> > +};
> > +
> > +static int mx25f0a_nand_probe(struct platform_device *pdev)
> > +{
> > +   struct mtd_info *mtd;
> > +   struct mx25f0a_mfd *mfd = dev_get_drvdata(pdev->dev.parent);
> > +   struct mxic_nand_ctlr *mxic;
> > +   struct nand_chip *nand_chip;
> > +   int err;
> > +
> > +   mxic = devm_kzalloc(&pdev->dev, sizeof(struct mxic_nand_ctlr),
> > +             GFP_KERNEL);
> 
> mxic for a NAND controller structure is probably not a name meaningful
> enough.

How about *fmc or *mxic_fmc ?

> 
> > +   if (!mxic)
> > +      return -ENOMEM;
> > +
> > +   nand_chip = &mxic->nand;
> > +   mtd = nand_to_mtd(nand_chip);
> > +   mtd->dev.parent = pdev->dev.parent;
> > +   nand_chip->ecc.priv = NULL;
> > +   nand_set_flash_node(nand_chip, pdev->dev.parent->of_node);
> > +   nand_chip->priv = mxic;
> > +
> > +   mxic->mfd = mfd;
> > +
> > +   nand_chip->legacy.select_chip = mxic_nand_select_chip;
> 
> Please don't implement legacy interfaces. You can check in
> marvell_nand.c how this is handled now:
> 
> b25251414f6e mtd: rawnand: marvell: Stop implementing ->select_chip()

got it, will check & patch.

> 
> > +   mxic->base.ops = &mxic_nand_controller_ops;
> > +   nand_controller_init(&mxic->base);
> > +   nand_chip->controller = &mxic->base;
> > +
> > +   mxic_host_init(mxic);
> > +
> > +   err = nand_scan(nand_chip, 1);
> > +   if (err)
> > +      goto fail;
> 
> You can return directly as there is nothing to clean in the error path.
> 
> > +
> > +   err = mtd_device_register(mtd, NULL, 0);
> > +   if (err)
> > +      goto fail;
> 
> Ditto
> 
> > +
> > +   platform_set_drvdata(pdev, mxic);
> > +
> > +   return 0;
> > +fail:
> > +   return err;
> > +}
> > +
> > +static int mx25f0a_nand_remove(struct platform_device *pdev)
> > +{
> > +   struct mxic_nand_ctlr *mxic = platform_get_drvdata(pdev);
> > +
> > +   nand_release(&mxic->nand);
> > +
> > +   return 0;
> > +}
> > +
> > +static struct platform_driver mx25f0a_nand_driver = {
> > +   .probe      = mx25f0a_nand_probe,
> 
> Please don't align '=' on tabs.

okay, will fix and also remove "mx25f0a" char.
patch to:
mxic_nand_driver & mxic_nand_probe.


thanks for your review.
best regards,
Mason

--






CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information 
and/or personal data, which is protected by applicable laws. Please be 
reminded that duplication, disclosure, distribution, or use of this e-mail 
(and/or its attachments) or any part thereof is prohibited. If you receive 
this e-mail in error, please notify us immediately and delete this mail as 
well as its attachment(s) from your system. In addition, please be 
informed that collection, processing, and/or use of personal data is 
prohibited unless expressly permitted by personal data protection laws. 
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================



============================================================================

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================


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

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

* Re: [PATCH v3 2/4] mtd: rawnand: Add Macronix MX25F0A NAND controller
       [not found]     ` <OF8A566F14.A2F0F576-ON482583FB.002E7E32-482583FB.003068B1@LocalDomain>
@ 2019-05-15  9:18       ` masonccyang
  0 siblings, 0 replies; 21+ messages in thread
From: masonccyang @ 2019-05-15  9:18 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: mark.rutland, christophe.kerello, marcel.ziswiler, stefan,
	liang.yang, linux-mtd, Miquel Raynal, lee.jones, richard,
	marek.vasut, geert, devicetree, robh+dt, bbrezillon, juliensu,
	linux-kernel, linux-spi, paul.burton, broonie, computersforpeace,
	dwmw2, zhengxunli


Hi Miquel,

Sorry, previous email missed this mxic_nand_data_xfer() reply.

This Flash Memory Controller implemented the Buffer read-write data 
transfer 
for SPI mode and raw NAND mode.

That is each time driver write to the transmit of the TXD port and the 
data 
is shifted out, new data is received in RXD port. 
By transmitting the entire buffer without reading any data, driver are 
losing
the received data.

Actually the mxic_nand_data_xfer() is a copy of mxic_spi_data_xfer().

https://github.com/torvalds/linux/blame/master/drivers/spi/spi-mxic.c 

therefore, I plan to patch this part to MFD as a common code for 
both raw NAND and SPI.

i.e,. 
In driver/mfd/mxic-mfd.c, we have mxic_mfd_data_xfer() and 

here call mxic_mfd_data_xfer() for raw NAND data read-write.


> > > +static int mxic_nand_data_xfer(struct mxic_nand_ctlr *mxic, const 
void *txbuf,
> > > +                void *rxbuf, unsigned int len)
> > > +{
> > 
> > There is not so much code shared, why not separating this function for
> > rx and tx cases?
> > 
> > > +   unsigned int pos = 0;
> > > +
> > > +   while (pos < len) {
> > > +      unsigned int nbytes = len - pos;
> > > +      u32 data = 0xffffffff;
> > > +      u32 sts;
> > > +      int ret;
> > > +
> > > +      if (nbytes > 4)
> > > +         nbytes = 4;
> > > +
> > > +      if (txbuf)
> > > +         memcpy(&data, txbuf + pos, nbytes);
> > > +
> > > +      ret = readl_poll_timeout(mxic->mfd->regs + INT_STS, sts,
> > > +                sts & INT_TX_EMPTY, 0, USEC_PER_SEC);
> > 
> > Using USEC_PER_SEC for a delay is weird
> > 
> > > +      if (ret)
> > > +         return ret;
> > > +
> > > +      writel(data, mxic->mfd->regs + TXD(nbytes % 4));
> > > +
> > > +      if (rxbuf) {
> > > +         ret = readl_poll_timeout(mxic->mfd->regs + INT_STS, sts,
> > > +                   sts & INT_TX_EMPTY, 0,
> > > +                   USEC_PER_SEC);
> > > +         if (ret)
> > > +            return ret;
> > > +
> > > +         ret = readl_poll_timeout(mxic->mfd->regs + INT_STS, sts,
> > > +                   sts & INT_RX_NOT_EMPTY, 0,
> > > +                   USEC_PER_SEC);
> > > +         if (ret)
> > > +            return ret;
> > > +
> > > +         data = readl(mxic->mfd->regs + RXD);
> > > +         data >>= (8 * (4 - nbytes));
> > 
> > What is this? Are you trying to handle some endianness issue?
> 
> yes,
> 
> > 
> > > +         memcpy(rxbuf + pos, &data, nbytes);
> > > +         WARN_ON(readl(mxic->mfd->regs + INT_STS) &
> > > +            INT_RX_NOT_EMPTY);
> > > +      } else {
> > > +         readl(mxic->mfd->regs + RXD);
> > > +      }
> > > +      WARN_ON(readl(mxic->mfd->regs + INT_STS) & INT_RX_NOT_EMPTY);
> > > +
> > > +      pos += nbytes;
> > > +   }
> > > +
> > > +   return 0;
> > > +}

thanks for your review.

best regards,
Mason

--

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information 
and/or personal data, which is protected by applicable laws. Please be 
reminded that duplication, disclosure, distribution, or use of this e-mail 
(and/or its attachments) or any part thereof is prohibited. If you receive 
this e-mail in error, please notify us immediately and delete this mail as 
well as its attachment(s) from your system. In addition, please be 
informed that collection, processing, and/or use of personal data is 
prohibited unless expressly permitted by personal data protection laws. 
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================



============================================================================

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================


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

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

* Re: [PATCH v3 2/4] mtd: rawnand: Add Macronix MX25F0A NAND controller
  2019-05-15  8:48     ` masonccyang
@ 2019-05-15 12:08       ` Miquel Raynal
  0 siblings, 0 replies; 21+ messages in thread
From: Miquel Raynal @ 2019-05-15 12:08 UTC (permalink / raw)
  To: masonccyang
  Cc: mark.rutland, devicetree, christophe.kerello, bbrezillon,
	juliensu, lee.jones, linux-kernel, robh+dt, linux-spi,
	marcel.ziswiler, paul.burton, broonie, geert, stefan, linux-mtd,
	richard, liang.yang, computersforpeace, dwmw2, marek.vasut,
	zhengxunli

Hi masonccyang@mxic.com.tw,

masonccyang@mxic.com.tw wrote on Wed, 15 May 2019 16:48:46 +0800:

> Hi Miquel,
> 
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +//
> > > +// Copyright (C) 2019 Macronix International Co., Ltd.
> > > +//
> > > +// Authors:
> > > +//   Mason Yang <masonccyang@mxic.com.tw>
> > > +//   zhengxunli <zhengxunli@mxic.com.tw>  
> > 
> > This is not a valid name.
> > 
> > Also if he appears here I suppose he should be credited in the
> > module_authors() macro too.  
> 
> I think Li should maintain this NAND driver later, 

This entry is for the authors of the driver.

If he will maintain the driver, then add a new entry in MAINTAINERS.

> > > +}
> > > +
> > > +static const struct nand_controller_ops mxic_nand_controller_ops = {
> > > +   .exec_op = mxic_nand_exec_op,
> > > +};
> > > +
> > > +static int mx25f0a_nand_probe(struct platform_device *pdev)
> > > +{
> > > +   struct mtd_info *mtd;
> > > +   struct mx25f0a_mfd *mfd = dev_get_drvdata(pdev->dev.parent);
> > > +   struct mxic_nand_ctlr *mxic;
> > > +   struct nand_chip *nand_chip;
> > > +   int err;
> > > +
> > > +   mxic = devm_kzalloc(&pdev->dev, sizeof(struct mxic_nand_ctlr),
> > > +             GFP_KERNEL);  
> > 
> > mxic for a NAND controller structure is probably not a name meaningful
> > enough.  
> 
> How about *fmc or *mxic_fmc ?

fmc is fine, even if I personally prefer nfc for NAND flash controller.
Here the 'm' in fmc stands for 'memory' but I am not sure if the
controller can manage something else than NAND flash anyway?


Thanks,
Miquèl

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

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

* Re: [PATCH v3 2/4] mtd: rawnand: Add Macronix MX25F0A NAND controller
  2019-05-12 13:18   ` Miquel Raynal
  2019-05-15  8:48     ` masonccyang
       [not found]     ` <OF8A566F14.A2F0F576-ON482583FB.002E7E32-482583FB.003068B1@LocalDomain>
@ 2019-05-17  9:30     ` masonccyang
  2019-05-20 12:23       ` Miquel Raynal
  2 siblings, 1 reply; 21+ messages in thread
From: masonccyang @ 2019-05-17  9:30 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: mark.rutland, devicetree, christophe.kerello, bbrezillon,
	juliensu, lee.jones, linux-kernel, robh+dt, linux-spi,
	marcel.ziswiler, paul.burton, broonie, geert, stefan, linux-mtd,
	richard, liang.yang, computersforpeace, dwmw2, marek.vasut,
	zhengxunli


Hi Miquel,

> > +
> > +static void mxic_nand_select_chip(struct nand_chip *chip, int chipnr)
> 
> _select_target() is preferred now

Do you mean I implement mxic_nand_select_target() to control #CS ?

If so, I need to call mxic_nand_select_target( ) to control #CS ON
and then #CS OFF in _exec_op() due to nand_select_target()<in nand_base,c>
is still calling chip->legacy.select_chip ?

> 
> > +{
> > +   struct mxic_nand_ctlr *mxic = nand_get_controller_data(chip);
> > +
> > +   switch (chipnr) {
> > +   case 0:
> > +   case 1:
> > +      writel(HC_EN_BIT, mxic->mfd->regs + HC_EN);
> > +      writel(HC_CFG_MAN_CS_ASSERT | readl(mxic->mfd->regs + HC_CFG),
> > +             mxic->mfd->regs + HC_CFG);
> 
> In both case I would prefer a:
> 
>         reg = readl(...);
>         reg &= ~xxx;
>    reg |= yyy;
>    writel(reg, ...);
> 
> Much easier to read.
> 
> > +      break;
> > +
> > +   case -1:
> > +      writel(~HC_CFG_MAN_CS_ASSERT & readl(mxic->mfd->regs + HC_CFG),
> > +             mxic->mfd->regs + HC_CFG);
> > +      writel(0, mxic->mfd->regs + HC_EN);
> > +      break;
> > +
> > +   default:
> 
> Error?
> 
> > +      break;
> > +   }
> > +}
> > +

> > +static int mx25f0a_nand_probe(struct platform_device *pdev)
> > +{
> > +   struct mtd_info *mtd;
> > +   struct mx25f0a_mfd *mfd = dev_get_drvdata(pdev->dev.parent);
> > +   struct mxic_nand_ctlr *mxic;
> > +   struct nand_chip *nand_chip;
> > +   int err;
> > +
> > +   mxic = devm_kzalloc(&pdev->dev, sizeof(struct mxic_nand_ctlr),
> > +             GFP_KERNEL);
> 
> mxic for a NAND controller structure is probably not a name meaningful
> enough.
> 
> > +   if (!mxic)
> > +      return -ENOMEM;
> > +
> > +   nand_chip = &mxic->nand;
> > +   mtd = nand_to_mtd(nand_chip);
> > +   mtd->dev.parent = pdev->dev.parent;
> > +   nand_chip->ecc.priv = NULL;
> > +   nand_set_flash_node(nand_chip, pdev->dev.parent->of_node);
> > +   nand_chip->priv = mxic;
> > +
> > +   mxic->mfd = mfd;
> > +
> > +   nand_chip->legacy.select_chip = mxic_nand_select_chip;
> 
> Please don't implement legacy interfaces. You can check in
> marvell_nand.c how this is handled now:
> 
> b25251414f6e mtd: rawnand: marvell: Stop implementing ->select_chip()
> 

Does it mean chip->legacy.select_chip() will phase-out ?


thanks & best regards,
Mason

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information 
and/or personal data, which is protected by applicable laws. Please be 
reminded that duplication, disclosure, distribution, or use of this e-mail 
(and/or its attachments) or any part thereof is prohibited. If you receive 
this e-mail in error, please notify us immediately and delete this mail as 
well as its attachment(s) from your system. In addition, please be 
informed that collection, processing, and/or use of personal data is 
prohibited unless expressly permitted by personal data protection laws. 
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================



============================================================================

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================


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

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

* Re: [PATCH v3 2/4] mtd: rawnand: Add Macronix MX25F0A NAND controller
  2019-05-17  9:30     ` masonccyang
@ 2019-05-20 12:23       ` Miquel Raynal
  2019-05-23  8:58         ` masonccyang
  0 siblings, 1 reply; 21+ messages in thread
From: Miquel Raynal @ 2019-05-20 12:23 UTC (permalink / raw)
  To: masonccyang
  Cc: mark.rutland, devicetree, christophe.kerello, bbrezillon,
	juliensu, lee.jones, linux-kernel, robh+dt, linux-spi,
	marcel.ziswiler, paul.burton, broonie, geert, stefan, linux-mtd,
	richard, liang.yang, computersforpeace, dwmw2, marek.vasut,
	zhengxunli

Hi Mason,

masonccyang@mxic.com.tw wrote on Fri, 17 May 2019 17:30:21 +0800:

> Hi Miquel,
> 
> > > +
> > > +static void mxic_nand_select_chip(struct nand_chip *chip, int chipnr)  
> > 
> > _select_target() is preferred now  
> 
> Do you mean I implement mxic_nand_select_target() to control #CS ?
> 
> If so, I need to call mxic_nand_select_target( ) to control #CS ON
> and then #CS OFF in _exec_op() due to nand_select_target()<in nand_base,c>
> is still calling chip->legacy.select_chip ?

You must forget about the ->select_chip() callback. Now it should be
handled directly from the controller driver. Please have a look at the
commit pointed against the marvell_nand.c driver.

[...]

> > > +   if (!mxic)
> > > +      return -ENOMEM;
> > > +
> > > +   nand_chip = &mxic->nand;
> > > +   mtd = nand_to_mtd(nand_chip);
> > > +   mtd->dev.parent = pdev->dev.parent;
> > > +   nand_chip->ecc.priv = NULL;
> > > +   nand_set_flash_node(nand_chip, pdev->dev.parent->of_node);
> > > +   nand_chip->priv = mxic;
> > > +
> > > +   mxic->mfd = mfd;
> > > +
> > > +   nand_chip->legacy.select_chip = mxic_nand_select_chip;  
> > 
> > Please don't implement legacy interfaces. You can check in
> > marvell_nand.c how this is handled now:
> > 
> > b25251414f6e mtd: rawnand: marvell: Stop implementing ->select_chip()
> >   
> 
> Does it mean chip->legacy.select_chip() will phase-out ?

Yes it will.

Thanks,
Miquèl

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

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

* Re: [PATCH v3 2/4] mtd: rawnand: Add Macronix MX25F0A NAND controller
  2019-05-20 12:23       ` Miquel Raynal
@ 2019-05-23  8:58         ` masonccyang
  2019-05-27 12:42           ` Miquel Raynal
  0 siblings, 1 reply; 21+ messages in thread
From: masonccyang @ 2019-05-23  8:58 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: mark.rutland, devicetree, christophe.kerello, bbrezillon,
	juliensu, lee.jones, linux-kernel, robh+dt, linux-spi,
	marcel.ziswiler, paul.burton, broonie, geert, stefan, linux-mtd,
	richard, liang.yang, computersforpeace, dwmw2, marek.vasut,
	zhengxunli


Hi Miquel,

> > 
> > > > +
> > > > +static void mxic_nand_select_chip(struct nand_chip *chip, int 
chipnr) 
> > > 
> > > _select_target() is preferred now 
> > 
> > Do you mean I implement mxic_nand_select_target() to control #CS ?
> > 
> > If so, I need to call mxic_nand_select_target( ) to control #CS ON
> > and then #CS OFF in _exec_op() due to nand_select_target()<in 
nand_base,c>
> > is still calling chip->legacy.select_chip ?
> 
> You must forget about the ->select_chip() callback. Now it should be
> handled directly from the controller driver. Please have a look at the
> commit pointed against the marvell_nand.c driver.

I have no Marvell NFC datasheet and have one question.

In marvell_nand.c, there is no xxx_deselect_target() or 
something like that doing #CS OFF.
marvell_nfc_select_target() seems always to make one of chip or die
#CS keep low.

Is it right ?

How to make all #CS keep high for NAND to enter 
low-power standby mode if driver don't use "legacy.select_chip()" ?

thanks & best regards,
Mason

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information 
and/or personal data, which is protected by applicable laws. Please be 
reminded that duplication, disclosure, distribution, or use of this e-mail 
(and/or its attachments) or any part thereof is prohibited. If you receive 
this e-mail in error, please notify us immediately and delete this mail as 
well as its attachment(s) from your system. In addition, please be 
informed that collection, processing, and/or use of personal data is 
prohibited unless expressly permitted by personal data protection laws. 
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================



============================================================================

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================


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

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

* Re: [PATCH v3 2/4] mtd: rawnand: Add Macronix MX25F0A NAND controller
  2019-05-23  8:58         ` masonccyang
@ 2019-05-27 12:42           ` Miquel Raynal
  2019-05-29  3:12             ` masonccyang
  0 siblings, 1 reply; 21+ messages in thread
From: Miquel Raynal @ 2019-05-27 12:42 UTC (permalink / raw)
  To: masonccyang
  Cc: mark.rutland, devicetree, christophe.kerello, bbrezillon,
	juliensu, lee.jones, linux-kernel, robh+dt, linux-spi,
	marcel.ziswiler, paul.burton, broonie, geert, stefan, linux-mtd,
	richard, liang.yang, computersforpeace, dwmw2, marek.vasut,
	zhengxunli

Hi Mason,

masonccyang@mxic.com.tw wrote on Thu, 23 May 2019 16:58:02 +0800:

> Hi Miquel,
> 
> > >   
> > > > > +
> > > > > +static void mxic_nand_select_chip(struct nand_chip *chip, int   
> chipnr) 
> > > > 
> > > > _select_target() is preferred now   
> > > 
> > > Do you mean I implement mxic_nand_select_target() to control #CS ?
> > > 
> > > If so, I need to call mxic_nand_select_target( ) to control #CS ON
> > > and then #CS OFF in _exec_op() due to nand_select_target()<in   
> nand_base,c>  
> > > is still calling chip->legacy.select_chip ?  
> > 
> > You must forget about the ->select_chip() callback. Now it should be
> > handled directly from the controller driver. Please have a look at the
> > commit pointed against the marvell_nand.c driver.  
> 
> I have no Marvell NFC datasheet and have one question.
> 
> In marvell_nand.c, there is no xxx_deselect_target() or 
> something like that doing #CS OFF.
> marvell_nfc_select_target() seems always to make one of chip or die
> #CS keep low.
> 
> Is it right ?

Yes, AFAIR there is no "de-assert" mechanism in this controller.

> 
> How to make all #CS keep high for NAND to enter 
> low-power standby mode if driver don't use "legacy.select_chip()" ?

See commit 02b4a52604a4 ("mtd: rawnand: Make ->select_chip() optional
when ->exec_op() is implemented") which states:

        "When [->select_chip() is] not implemented, the core is assuming
	the CS line is automatically asserted/deasserted by the driver
	->exec_op() implementation."

Of course, the above is right only when the controller driver supports
the ->exec_op() interface. 

So if you think it is not too time consuming and worth the trouble to
assert/deassert the CS at each operation, you may do it in your driver.


Thanks,
Miquèl

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

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

* Re: [PATCH v3 2/4] mtd: rawnand: Add Macronix MX25F0A NAND controller
  2019-05-27 12:42           ` Miquel Raynal
@ 2019-05-29  3:12             ` masonccyang
  0 siblings, 0 replies; 21+ messages in thread
From: masonccyang @ 2019-05-29  3:12 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: mark.rutland, devicetree, christophe.kerello, bbrezillon,
	juliensu, lee.jones, linux-kernel, robh+dt, linux-spi,
	marcel.ziswiler, paul.burton, broonie, geert, stefan, linux-mtd,
	richard, liang.yang, computersforpeace, dwmw2, marek.vasut,
	zhengxunli


Hi Miquel,

> > > > > > +static void mxic_nand_select_chip(struct nand_chip *chip, int 
 
> > chipnr) 
> > > > > 
> > > > > _select_target() is preferred now 
> > > > 
> > > > Do you mean I implement mxic_nand_select_target() to control #CS ?
> > > > 
> > > > If so, I need to call mxic_nand_select_target( ) to control #CS ON
> > > > and then #CS OFF in _exec_op() due to nand_select_target()<in 
> > nand_base,c> 
> > > > is still calling chip->legacy.select_chip ? 
> > > 
> > > You must forget about the ->select_chip() callback. Now it should be
> > > handled directly from the controller driver. Please have a look at 
the
> > > commit pointed against the marvell_nand.c driver. 
> > 
> > I have no Marvell NFC datasheet and have one question.
> > 
> > In marvell_nand.c, there is no xxx_deselect_target() or 
> > something like that doing #CS OFF.
> > marvell_nfc_select_target() seems always to make one of chip or die
> > #CS keep low.
> > 
> > Is it right ?
> 
> Yes, AFAIR there is no "de-assert" mechanism in this controller.
> 
> > 
> > How to make all #CS keep high for NAND to enter 
> > low-power standby mode if driver don't use "legacy.select_chip()" ?
> 
> See commit 02b4a52604a4 ("mtd: rawnand: Make ->select_chip() optional
> when ->exec_op() is implemented") which states:
> 
>         "When [->select_chip() is] not implemented, the core is assuming
>    the CS line is automatically asserted/deasserted by the driver
>    ->exec_op() implementation."
> 
> Of course, the above is right only when the controller driver supports
> the ->exec_op() interface. 

Currently, it seems that we will get the incorrect data and error
operation due to CS in error toggling if CS line is controlled in 
->exec_op().
i.e,. 

1) In nand_onfi_detect() to call nand_exec_op() twice by 
nand_read_param_page_op() and annd_read_data_op()

2) In nand_write_page_xxx to call nand_exec_op() many times by
nand_prog_page_begin_op(), nand_write_data_op() and 
nand_prog_page_end_op().


Should we consider to add a CS line controller in struct nand_controller
i.e,.

struct nand_controller {
         struct mutex lock;
         const struct nand_controller_ops *ops;
+          void (*select_chip)(struct nand_chip *chip, int cs);
};

to replace legacy.select_chip() ?


To patch in nand_select_target() and nand_deselect_target()

void nand_select_target(struct nand_chip *chip, unsigned int cs)
{
        /*
         * cs should always lie between 0 and chip->numchips, when that's 
not
         * the case it's a bug and the caller should be fixed.
         */
        if (WARN_ON(cs > chip->numchips))
                return;

        chip->cur_cs = cs;

+       if (chip->controller->select_chip)
+               chip->controller->select_chip(chip, cs);
+
        if (chip->legacy.select_chip)
                chip->legacy.select_chip(chip, cs);
}

void nand_deselect_target(struct nand_chip *chip)
{
+       if (chip->controller->select_chip)
+               chip->controller->select_chip(chip, -1);
+
        if (chip->legacy.select_chip)
                chip->legacy.select_chip(chip, -1);

        chip->cur_cs = -1;
}


> 
> So if you think it is not too time consuming and worth the trouble to
> assert/deassert the CS at each operation, you may do it in your driver.
> 
> 
> Thanks,
> Miquèl

thanks & best regards,
Mason

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information 
and/or personal data, which is protected by applicable laws. Please be 
reminded that duplication, disclosure, distribution, or use of this e-mail 
(and/or its attachments) or any part thereof is prohibited. If you receive 
this e-mail in error, please notify us immediately and delete this mail as 
well as its attachment(s) from your system. In addition, please be 
informed that collection, processing, and/or use of personal data is 
prohibited unless expressly permitted by personal data protection laws. 
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================



============================================================================

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================


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

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

end of thread, back to index

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-15  9:23 [PATCH v3 0/4] Add Macronix MX25F0A MFD driver for raw nand and spi Mason Yang
2019-04-15  9:23 ` [PATCH v3 1/4] mfd: Add Macronix MX25F0A MFD controller driver Mason Yang
2019-05-12 13:08   ` Miquel Raynal
2019-05-15  6:46     ` masonccyang
2019-04-15  9:23 ` [PATCH v3 2/4] mtd: rawnand: Add Macronix MX25F0A NAND controller Mason Yang
2019-05-12 13:18   ` Miquel Raynal
2019-05-15  8:48     ` masonccyang
2019-05-15 12:08       ` Miquel Raynal
     [not found]     ` <OF8A566F14.A2F0F576-ON482583FB.002E7E32-482583FB.003068B1@LocalDomain>
2019-05-15  9:18       ` masonccyang
2019-05-17  9:30     ` masonccyang
2019-05-20 12:23       ` Miquel Raynal
2019-05-23  8:58         ` masonccyang
2019-05-27 12:42           ` Miquel Raynal
2019-05-29  3:12             ` masonccyang
2019-04-15  9:23 ` [PATCH v3 3/4] spi: Patch Macronix SPI controller driver according to MX25F0A MFD driver Mason Yang
2019-04-19 14:51   ` Mark Brown
     [not found]     ` <OF7742B4A9.445066F6-ON482583EC.0037E377-482583EC.0039125B@mxic.com.tw>
2019-05-02  2:41       ` Mark Brown
2019-04-15  9:23 ` [PATCH v3 4/4] dt-bindings: mfd: Document Macronix MX25F0A controller bindings Mason Yang
2019-04-26 22:41   ` Rob Herring
2019-05-12 13:23   ` Miquel Raynal
2019-05-15  7:36     ` masonccyang

Linux-mtd Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mtd/0 linux-mtd/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-mtd linux-mtd/ https://lore.kernel.org/linux-mtd \
		linux-mtd@lists.infradead.org linux-mtd@archiver.kernel.org
	public-inbox-index linux-mtd


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-mtd


AGPL code for this site: git clone https://public-inbox.org/ public-inbox