All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] mtd: nand: raw: brcmnand: Refactored code and introduced inline functions
@ 2019-05-30 21:20 ` Kamal Dasu
  0 siblings, 0 replies; 14+ messages in thread
From: Kamal Dasu @ 2019-05-30 21:20 UTC (permalink / raw)
  To: linux-mtd
  Cc: bcm-kernel-feedback-list, linux-kernel, Kamal Dasu, Brian Norris,
	Miquel Raynal, Richard Weinberger, David Woodhouse, Marek Vasut,
	Vignesh Raghavendra

Refactored NAND ECC and CMD address configuration code to use inline
functions.

Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
---
 drivers/mtd/nand/raw/brcmnand/brcmnand.c | 100 +++++++++++++++++++------------
 1 file changed, 62 insertions(+), 38 deletions(-)

diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
index ce0b8ff..77b7850 100644
--- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
@@ -588,6 +588,54 @@ static inline void brcmnand_write_fc(struct brcmnand_controller *ctrl,
 	__raw_writel(val, ctrl->nand_fc + word * 4);
 }
 
+static inline void brcmnand_clear_ecc_addr(struct brcmnand_controller *ctrl)
+{
+
+	/* Clear error addresses */
+	brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_ADDR, 0);
+	brcmnand_write_reg(ctrl, BRCMNAND_CORR_ADDR, 0);
+	brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_EXT_ADDR, 0);
+	brcmnand_write_reg(ctrl, BRCMNAND_CORR_EXT_ADDR, 0);
+}
+
+static inline u64 brcmnand_get_uncorrecc_addr(struct brcmnand_controller *ctrl)
+{
+	u64 err_addr;
+
+	err_addr = brcmnand_read_reg(ctrl, BRCMNAND_UNCORR_ADDR);
+	err_addr |= ((u64)(brcmnand_read_reg(ctrl,
+					     BRCMNAND_UNCORR_EXT_ADDR)
+					     & 0xffff) << 32);
+
+	return err_addr;
+}
+
+static inline u64 brcmnand_get_correcc_addr(struct brcmnand_controller *ctrl)
+{
+	u64 err_addr;
+
+	err_addr = brcmnand_read_reg(ctrl, BRCMNAND_CORR_ADDR);
+	err_addr |= ((u64)(brcmnand_read_reg(ctrl,
+					     BRCMNAND_CORR_EXT_ADDR)
+					     & 0xffff) << 32);
+
+	return err_addr;
+}
+
+static inline void brcmnand_set_cmd_addr(struct mtd_info *mtd, u64 addr)
+{
+	struct nand_chip *chip =  mtd_to_nand(mtd);
+	struct brcmnand_host *host = nand_get_controller_data(chip);
+	struct brcmnand_controller *ctrl = host->ctrl;
+
+	brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS,
+			   (host->cs << 16) | ((addr >> 32) & 0xffff));
+	(void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS);
+	brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS,
+			   lower_32_bits(addr));
+	(void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS);
+}
+
 static inline u16 brcmnand_cs_offset(struct brcmnand_controller *ctrl, int cs,
 				     enum brcmnand_cs_reg reg)
 {
@@ -1213,9 +1261,12 @@ static void brcmnand_send_cmd(struct brcmnand_host *host, int cmd)
 {
 	struct brcmnand_controller *ctrl = host->ctrl;
 	int ret;
+	u64 cmd_addr;
+
+	cmd_addr = brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS);
+
+	dev_dbg(ctrl->dev, "send native cmd %d addr 0x%llx\n", cmd, cmd_addr);
 
-	dev_dbg(ctrl->dev, "send native cmd %d addr_lo 0x%x\n", cmd,
-		brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS));
 	BUG_ON(ctrl->cmd_pending != 0);
 	ctrl->cmd_pending = cmd;
 
@@ -1374,12 +1425,7 @@ static void brcmnand_cmdfunc(struct nand_chip *chip, unsigned command,
 	if (!native_cmd)
 		return;
 
-	brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS,
-		(host->cs << 16) | ((addr >> 32) & 0xffff));
-	(void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS);
-	brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS, lower_32_bits(addr));
-	(void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS);
-
+	brcmnand_set_cmd_addr(mtd, addr);
 	brcmnand_send_cmd(host, native_cmd);
 	brcmnand_waitfunc(chip);
 
@@ -1597,20 +1643,10 @@ static int brcmnand_read_by_pio(struct mtd_info *mtd, struct nand_chip *chip,
 	struct brcmnand_controller *ctrl = host->ctrl;
 	int i, j, ret = 0;
 
-	/* Clear error addresses */
-	brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_ADDR, 0);
-	brcmnand_write_reg(ctrl, BRCMNAND_CORR_ADDR, 0);
-	brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_EXT_ADDR, 0);
-	brcmnand_write_reg(ctrl, BRCMNAND_CORR_EXT_ADDR, 0);
-
-	brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS,
-			(host->cs << 16) | ((addr >> 32) & 0xffff));
-	(void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS);
+	brcmnand_clear_ecc_addr(ctrl);
 
 	for (i = 0; i < trans; i++, addr += FC_BYTES) {
-		brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS,
-				   lower_32_bits(addr));
-		(void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS);
+		brcmnand_set_cmd_addr(mtd, addr);
 		/* SPARE_AREA_READ does not use ECC, so just use PAGE_READ */
 		brcmnand_send_cmd(host, CMD_PAGE_READ);
 		brcmnand_waitfunc(chip);
@@ -1630,21 +1666,15 @@ static int brcmnand_read_by_pio(struct mtd_info *mtd, struct nand_chip *chip,
 					host->hwcfg.sector_size_1k);
 
 		if (!ret) {
-			*err_addr = brcmnand_read_reg(ctrl,
-					BRCMNAND_UNCORR_ADDR) |
-				((u64)(brcmnand_read_reg(ctrl,
-						BRCMNAND_UNCORR_EXT_ADDR)
-					& 0xffff) << 32);
+			*err_addr = brcmnand_get_uncorrecc_addr(ctrl);
+
 			if (*err_addr)
 				ret = -EBADMSG;
 		}
 
 		if (!ret) {
-			*err_addr = brcmnand_read_reg(ctrl,
-					BRCMNAND_CORR_ADDR) |
-				((u64)(brcmnand_read_reg(ctrl,
-						BRCMNAND_CORR_EXT_ADDR)
-					& 0xffff) << 32);
+			*err_addr = brcmnand_get_correcc_addr(ctrl);
+
 			if (*err_addr)
 				ret = -EUCLEAN;
 		}
@@ -1711,7 +1741,7 @@ static int brcmnand_read(struct mtd_info *mtd, struct nand_chip *chip,
 	dev_dbg(ctrl->dev, "read %llx -> %p\n", (unsigned long long)addr, buf);
 
 try_dmaread:
-	brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_COUNT, 0);
+	brcmnand_clear_ecc_addr(ctrl);
 
 	if (has_flash_dma(ctrl) && !oob && flash_dma_buf_ok(buf)) {
 		err = brcmnand_dma_trans(host, addr, buf, trans * FC_BYTES,
@@ -1858,15 +1888,9 @@ static int brcmnand_write(struct mtd_info *mtd, struct nand_chip *chip,
 		goto out;
 	}
 
-	brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS,
-			(host->cs << 16) | ((addr >> 32) & 0xffff));
-	(void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS);
-
 	for (i = 0; i < trans; i++, addr += FC_BYTES) {
 		/* full address MUST be set before populating FC */
-		brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS,
-				   lower_32_bits(addr));
-		(void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS);
+		brcmnand_set_cmd_addr(mtd, addr);
 
 		if (buf) {
 			brcmnand_soc_data_bus_prepare(ctrl->soc, false);
-- 
1.9.0.138.g2de3478


^ permalink raw reply related	[flat|nested] 14+ messages in thread
* [PATCH 1/3] mtd: nand: raw: brcmnand: Refactored code and introduced inline functions
@ 2019-05-30 21:20 ` Kamal Dasu
  0 siblings, 0 replies; 14+ messages in thread
From: Kamal Dasu @ 2019-05-30 21:20 UTC (permalink / raw)
  To: linux-mtd
  Cc: Vignesh Raghavendra, Kamal Dasu, Richard Weinberger,
	linux-kernel, Marek Vasut, bcm-kernel-feedback-list,
	Miquel Raynal, Brian Norris, David Woodhouse

Refactored NAND ECC and CMD address configuration code to use inline
functions.

Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
---
 drivers/mtd/nand/raw/brcmnand/brcmnand.c | 100 +++++++++++++++++++------------
 1 file changed, 62 insertions(+), 38 deletions(-)

diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
index ce0b8ff..77b7850 100644
--- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
@@ -588,6 +588,54 @@ static inline void brcmnand_write_fc(struct brcmnand_controller *ctrl,
 	__raw_writel(val, ctrl->nand_fc + word * 4);
 }
 
+static inline void brcmnand_clear_ecc_addr(struct brcmnand_controller *ctrl)
+{
+
+	/* Clear error addresses */
+	brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_ADDR, 0);
+	brcmnand_write_reg(ctrl, BRCMNAND_CORR_ADDR, 0);
+	brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_EXT_ADDR, 0);
+	brcmnand_write_reg(ctrl, BRCMNAND_CORR_EXT_ADDR, 0);
+}
+
+static inline u64 brcmnand_get_uncorrecc_addr(struct brcmnand_controller *ctrl)
+{
+	u64 err_addr;
+
+	err_addr = brcmnand_read_reg(ctrl, BRCMNAND_UNCORR_ADDR);
+	err_addr |= ((u64)(brcmnand_read_reg(ctrl,
+					     BRCMNAND_UNCORR_EXT_ADDR)
+					     & 0xffff) << 32);
+
+	return err_addr;
+}
+
+static inline u64 brcmnand_get_correcc_addr(struct brcmnand_controller *ctrl)
+{
+	u64 err_addr;
+
+	err_addr = brcmnand_read_reg(ctrl, BRCMNAND_CORR_ADDR);
+	err_addr |= ((u64)(brcmnand_read_reg(ctrl,
+					     BRCMNAND_CORR_EXT_ADDR)
+					     & 0xffff) << 32);
+
+	return err_addr;
+}
+
+static inline void brcmnand_set_cmd_addr(struct mtd_info *mtd, u64 addr)
+{
+	struct nand_chip *chip =  mtd_to_nand(mtd);
+	struct brcmnand_host *host = nand_get_controller_data(chip);
+	struct brcmnand_controller *ctrl = host->ctrl;
+
+	brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS,
+			   (host->cs << 16) | ((addr >> 32) & 0xffff));
+	(void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS);
+	brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS,
+			   lower_32_bits(addr));
+	(void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS);
+}
+
 static inline u16 brcmnand_cs_offset(struct brcmnand_controller *ctrl, int cs,
 				     enum brcmnand_cs_reg reg)
 {
@@ -1213,9 +1261,12 @@ static void brcmnand_send_cmd(struct brcmnand_host *host, int cmd)
 {
 	struct brcmnand_controller *ctrl = host->ctrl;
 	int ret;
+	u64 cmd_addr;
+
+	cmd_addr = brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS);
+
+	dev_dbg(ctrl->dev, "send native cmd %d addr 0x%llx\n", cmd, cmd_addr);
 
-	dev_dbg(ctrl->dev, "send native cmd %d addr_lo 0x%x\n", cmd,
-		brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS));
 	BUG_ON(ctrl->cmd_pending != 0);
 	ctrl->cmd_pending = cmd;
 
@@ -1374,12 +1425,7 @@ static void brcmnand_cmdfunc(struct nand_chip *chip, unsigned command,
 	if (!native_cmd)
 		return;
 
-	brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS,
-		(host->cs << 16) | ((addr >> 32) & 0xffff));
-	(void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS);
-	brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS, lower_32_bits(addr));
-	(void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS);
-
+	brcmnand_set_cmd_addr(mtd, addr);
 	brcmnand_send_cmd(host, native_cmd);
 	brcmnand_waitfunc(chip);
 
@@ -1597,20 +1643,10 @@ static int brcmnand_read_by_pio(struct mtd_info *mtd, struct nand_chip *chip,
 	struct brcmnand_controller *ctrl = host->ctrl;
 	int i, j, ret = 0;
 
-	/* Clear error addresses */
-	brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_ADDR, 0);
-	brcmnand_write_reg(ctrl, BRCMNAND_CORR_ADDR, 0);
-	brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_EXT_ADDR, 0);
-	brcmnand_write_reg(ctrl, BRCMNAND_CORR_EXT_ADDR, 0);
-
-	brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS,
-			(host->cs << 16) | ((addr >> 32) & 0xffff));
-	(void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS);
+	brcmnand_clear_ecc_addr(ctrl);
 
 	for (i = 0; i < trans; i++, addr += FC_BYTES) {
-		brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS,
-				   lower_32_bits(addr));
-		(void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS);
+		brcmnand_set_cmd_addr(mtd, addr);
 		/* SPARE_AREA_READ does not use ECC, so just use PAGE_READ */
 		brcmnand_send_cmd(host, CMD_PAGE_READ);
 		brcmnand_waitfunc(chip);
@@ -1630,21 +1666,15 @@ static int brcmnand_read_by_pio(struct mtd_info *mtd, struct nand_chip *chip,
 					host->hwcfg.sector_size_1k);
 
 		if (!ret) {
-			*err_addr = brcmnand_read_reg(ctrl,
-					BRCMNAND_UNCORR_ADDR) |
-				((u64)(brcmnand_read_reg(ctrl,
-						BRCMNAND_UNCORR_EXT_ADDR)
-					& 0xffff) << 32);
+			*err_addr = brcmnand_get_uncorrecc_addr(ctrl);
+
 			if (*err_addr)
 				ret = -EBADMSG;
 		}
 
 		if (!ret) {
-			*err_addr = brcmnand_read_reg(ctrl,
-					BRCMNAND_CORR_ADDR) |
-				((u64)(brcmnand_read_reg(ctrl,
-						BRCMNAND_CORR_EXT_ADDR)
-					& 0xffff) << 32);
+			*err_addr = brcmnand_get_correcc_addr(ctrl);
+
 			if (*err_addr)
 				ret = -EUCLEAN;
 		}
@@ -1711,7 +1741,7 @@ static int brcmnand_read(struct mtd_info *mtd, struct nand_chip *chip,
 	dev_dbg(ctrl->dev, "read %llx -> %p\n", (unsigned long long)addr, buf);
 
 try_dmaread:
-	brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_COUNT, 0);
+	brcmnand_clear_ecc_addr(ctrl);
 
 	if (has_flash_dma(ctrl) && !oob && flash_dma_buf_ok(buf)) {
 		err = brcmnand_dma_trans(host, addr, buf, trans * FC_BYTES,
@@ -1858,15 +1888,9 @@ static int brcmnand_write(struct mtd_info *mtd, struct nand_chip *chip,
 		goto out;
 	}
 
-	brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS,
-			(host->cs << 16) | ((addr >> 32) & 0xffff));
-	(void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS);
-
 	for (i = 0; i < trans; i++, addr += FC_BYTES) {
 		/* full address MUST be set before populating FC */
-		brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS,
-				   lower_32_bits(addr));
-		(void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS);
+		brcmnand_set_cmd_addr(mtd, addr);
 
 		if (buf) {
 			brcmnand_soc_data_bus_prepare(ctrl->soc, false);
-- 
1.9.0.138.g2de3478


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

^ permalink raw reply related	[flat|nested] 14+ messages in thread
* [PATCH 2/3] mtd: nand: raw: brcmnand: Add support for v7.3 controller
  2019-05-30 21:20 ` Kamal Dasu
@ 2019-05-30 21:20   ` Kamal Dasu
  -1 siblings, 0 replies; 14+ messages in thread
From: Kamal Dasu @ 2019-05-30 21:20 UTC (permalink / raw)
  To: linux-mtd
  Cc: bcm-kernel-feedback-list, linux-kernel, Kamal Dasu, Brian Norris,
	Miquel Raynal, Richard Weinberger, David Woodhouse, Marek Vasut,
	Vignesh Raghavendra

This change adds support for brcm NAND v7.3 controller. This controller
uses a newer version of flash_dma engine and change mostly implements
these differences.

Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
---
 drivers/mtd/nand/raw/brcmnand/brcmnand.c | 102 ++++++++++++++++++++++++-------
 1 file changed, 80 insertions(+), 22 deletions(-)

diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
index 77b7850..544088f 100644
--- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
@@ -92,6 +92,12 @@ struct brcm_nand_dma_desc {
 #define FLASH_DMA_ECC_ERROR	(1 << 8)
 #define FLASH_DMA_CORR_ERROR	(1 << 9)
 
+/* Bitfields for DMA_MODE */
+#define FLASH_DMA_MODE_STOP_ON_ERROR	BIT(1) /* stop in Uncorr ECC error */
+#define FLASH_DMA_MODE_MODE		BIT(0) /* link list */
+#define FLASH_DMA_MODE_MASK		(FLASH_DMA_MODE_STOP_ON_ERROR |	\
+						FLASH_DMA_MODE_MODE)
+
 /* 512B flash cache in the NAND controller HW */
 #define FC_SHIFT		9U
 #define FC_BYTES		512U
@@ -104,6 +110,51 @@ struct brcm_nand_dma_desc {
 #define NAND_CTRL_RDY			(INTFC_CTLR_READY | INTFC_FLASH_READY)
 #define NAND_POLL_STATUS_TIMEOUT_MS	100
 
+/* flash_dma registers */
+enum flash_dma_reg {
+	FLASH_DMA_REVISION = 0,
+	FLASH_DMA_FIRST_DESC,
+	FLASH_DMA_FIRST_DESC_EXT,
+	FLASH_DMA_CTRL,
+	FLASH_DMA_MODE,
+	FLASH_DMA_STATUS,
+	FLASH_DMA_INTERRUPT_DESC,
+	FLASH_DMA_INTERRUPT_DESC_EXT,
+	FLASH_DMA_ERROR_STATUS,
+	FLASH_DMA_CURRENT_DESC,
+	FLASH_DMA_CURRENT_DESC_EXT,
+};
+
+/* flash_dma registers v1*/
+static const u16 flash_dma_regs_v1[] = {
+	[FLASH_DMA_REVISION]		= 0x00,
+	[FLASH_DMA_FIRST_DESC]		= 0x04,
+	[FLASH_DMA_FIRST_DESC_EXT]	= 0x08,
+	[FLASH_DMA_CTRL]		= 0x0c,
+	[FLASH_DMA_MODE]		= 0x10,
+	[FLASH_DMA_STATUS]		= 0x14,
+	[FLASH_DMA_INTERRUPT_DESC]	= 0x18,
+	[FLASH_DMA_INTERRUPT_DESC_EXT]	= 0x1c,
+	[FLASH_DMA_ERROR_STATUS]	= 0x20,
+	[FLASH_DMA_CURRENT_DESC]	= 0x24,
+	[FLASH_DMA_CURRENT_DESC_EXT]	= 0x28,
+};
+
+/* flash_dma registers v4 */
+static const u16 flash_dma_regs_v4[] = {
+	[FLASH_DMA_REVISION]		= 0x00,
+	[FLASH_DMA_FIRST_DESC]		= 0x08,
+	[FLASH_DMA_FIRST_DESC_EXT]	= 0x0c,
+	[FLASH_DMA_CTRL]		= 0x10,
+	[FLASH_DMA_MODE]		= 0x14,
+	[FLASH_DMA_STATUS]		= 0x18,
+	[FLASH_DMA_INTERRUPT_DESC]	= 0x20,
+	[FLASH_DMA_INTERRUPT_DESC_EXT]	= 0x24,
+	[FLASH_DMA_ERROR_STATUS]	= 0x28,
+	[FLASH_DMA_CURRENT_DESC]	= 0x30,
+	[FLASH_DMA_CURRENT_DESC_EXT]	= 0x34,
+};
+
 /* Controller feature flags */
 enum {
 	BRCMNAND_HAS_1K_SECTORS			= BIT(0),
@@ -136,6 +187,8 @@ struct brcmnand_controller {
 	/* List of NAND hosts (one for each chip-select) */
 	struct list_head host_list;
 
+	/* flash_dma reg */
+	const u16		*flash_dma_offsets;
 	struct brcm_nand_dma_desc *dma_desc;
 	dma_addr_t		dma_pa;
 
@@ -470,7 +523,7 @@ static int brcmnand_revision_init(struct brcmnand_controller *ctrl)
 	/* Register offsets */
 	if (ctrl->nand_version >= 0x0702)
 		ctrl->reg_offsets = brcmnand_regs_v72;
-	else if (ctrl->nand_version >= 0x0701)
+	else if (ctrl->nand_version == 0x0701)
 		ctrl->reg_offsets = brcmnand_regs_v71;
 	else if (ctrl->nand_version >= 0x0600)
 		ctrl->reg_offsets = brcmnand_regs_v60;
@@ -515,7 +568,7 @@ static int brcmnand_revision_init(struct brcmnand_controller *ctrl)
 	}
 
 	/* Maximum spare area sector size (per 512B) */
-	if (ctrl->nand_version >= 0x0702)
+	if (ctrl->nand_version == 0x0702)
 		ctrl->max_oob = 128;
 	else if (ctrl->nand_version >= 0x0600)
 		ctrl->max_oob = 64;
@@ -546,6 +599,15 @@ static int brcmnand_revision_init(struct brcmnand_controller *ctrl)
 	return 0;
 }
 
+static void brcmnand_flash_dma_revision_init(struct brcmnand_controller *ctrl)
+{
+	/* flash_dma register offsets */
+	if (ctrl->nand_version >= 0x0703)
+		ctrl->flash_dma_offsets = flash_dma_regs_v4;
+	else
+		ctrl->flash_dma_offsets = flash_dma_regs_v1;
+}
+
 static inline u32 brcmnand_read_reg(struct brcmnand_controller *ctrl,
 		enum brcmnand_reg reg)
 {
@@ -668,7 +730,7 @@ static void brcmnand_wr_corr_thresh(struct brcmnand_host *host, u8 val)
 	enum brcmnand_reg reg = BRCMNAND_CORR_THRESHOLD;
 	int cs = host->cs;
 
-	if (ctrl->nand_version >= 0x0702)
+	if (ctrl->nand_version == 0x0702)
 		bits = 7;
 	else if (ctrl->nand_version >= 0x0600)
 		bits = 6;
@@ -722,7 +784,7 @@ enum {
 
 static inline u32 brcmnand_spare_area_mask(struct brcmnand_controller *ctrl)
 {
-	if (ctrl->nand_version >= 0x0702)
+	if (ctrl->nand_version == 0x0702)
 		return GENMASK(7, 0);
 	else if (ctrl->nand_version >= 0x0600)
 		return GENMASK(6, 0);
@@ -852,20 +914,6 @@ static inline void brcmnand_set_wp(struct brcmnand_controller *ctrl, bool en)
  * Flash DMA
  ***********************************************************************/
 
-enum flash_dma_reg {
-	FLASH_DMA_REVISION		= 0x00,
-	FLASH_DMA_FIRST_DESC		= 0x04,
-	FLASH_DMA_FIRST_DESC_EXT	= 0x08,
-	FLASH_DMA_CTRL			= 0x0c,
-	FLASH_DMA_MODE			= 0x10,
-	FLASH_DMA_STATUS		= 0x14,
-	FLASH_DMA_INTERRUPT_DESC	= 0x18,
-	FLASH_DMA_INTERRUPT_DESC_EXT	= 0x1c,
-	FLASH_DMA_ERROR_STATUS		= 0x20,
-	FLASH_DMA_CURRENT_DESC		= 0x24,
-	FLASH_DMA_CURRENT_DESC_EXT	= 0x28,
-};
-
 static inline bool has_flash_dma(struct brcmnand_controller *ctrl)
 {
 	return ctrl->flash_dma_base;
@@ -877,14 +925,19 @@ static inline bool flash_dma_buf_ok(const void *buf)
 		likely(IS_ALIGNED((uintptr_t)buf, 4));
 }
 
-static inline void flash_dma_writel(struct brcmnand_controller *ctrl, u8 offs,
-				    u32 val)
+static inline void flash_dma_writel(struct brcmnand_controller *ctrl,
+				    enum flash_dma_reg dma_reg, u32 val)
 {
+	u16 offs = ctrl->flash_dma_offsets[dma_reg];
+
 	brcmnand_writel(val, ctrl->flash_dma_base + offs);
 }
 
-static inline u32 flash_dma_readl(struct brcmnand_controller *ctrl, u8 offs)
+static inline u32 flash_dma_readl(struct brcmnand_controller *ctrl,
+				  enum flash_dma_reg dma_reg)
 {
+	u16 offs = ctrl->flash_dma_offsets[dma_reg];
+
 	return brcmnand_readl(ctrl->flash_dma_base + offs);
 }
 
@@ -2427,6 +2480,7 @@ static int brcmnand_resume(struct device *dev)
 	{ .compatible = "brcm,brcmnand-v7.0" },
 	{ .compatible = "brcm,brcmnand-v7.1" },
 	{ .compatible = "brcm,brcmnand-v7.2" },
+	{ .compatible = "brcm,brcmnand-v7.3" },
 	{},
 };
 MODULE_DEVICE_TABLE(of, brcmnand_of_match);
@@ -2513,7 +2567,11 @@ int brcmnand_probe(struct platform_device *pdev, struct brcmnand_soc *soc)
 			goto err;
 		}
 
-		flash_dma_writel(ctrl, FLASH_DMA_MODE, 1); /* linked-list */
+		/* initialize the dma version */
+		brcmnand_flash_dma_revision_init(ctrl);
+
+		/* linked-list and stop on error */
+		flash_dma_writel(ctrl, FLASH_DMA_MODE, FLASH_DMA_MODE_MASK);
 		flash_dma_writel(ctrl, FLASH_DMA_ERROR_STATUS, 0);
 
 		/* Allocate descriptor(s) */
-- 
1.9.0.138.g2de3478


^ permalink raw reply related	[flat|nested] 14+ messages in thread
* [PATCH 2/3] mtd: nand: raw: brcmnand: Add support for v7.3 controller
@ 2019-05-30 21:20   ` Kamal Dasu
  0 siblings, 0 replies; 14+ messages in thread
From: Kamal Dasu @ 2019-05-30 21:20 UTC (permalink / raw)
  To: linux-mtd
  Cc: Vignesh Raghavendra, Kamal Dasu, Richard Weinberger,
	linux-kernel, Marek Vasut, bcm-kernel-feedback-list,
	Miquel Raynal, Brian Norris, David Woodhouse

This change adds support for brcm NAND v7.3 controller. This controller
uses a newer version of flash_dma engine and change mostly implements
these differences.

Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
---
 drivers/mtd/nand/raw/brcmnand/brcmnand.c | 102 ++++++++++++++++++++++++-------
 1 file changed, 80 insertions(+), 22 deletions(-)

diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
index 77b7850..544088f 100644
--- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
@@ -92,6 +92,12 @@ struct brcm_nand_dma_desc {
 #define FLASH_DMA_ECC_ERROR	(1 << 8)
 #define FLASH_DMA_CORR_ERROR	(1 << 9)
 
+/* Bitfields for DMA_MODE */
+#define FLASH_DMA_MODE_STOP_ON_ERROR	BIT(1) /* stop in Uncorr ECC error */
+#define FLASH_DMA_MODE_MODE		BIT(0) /* link list */
+#define FLASH_DMA_MODE_MASK		(FLASH_DMA_MODE_STOP_ON_ERROR |	\
+						FLASH_DMA_MODE_MODE)
+
 /* 512B flash cache in the NAND controller HW */
 #define FC_SHIFT		9U
 #define FC_BYTES		512U
@@ -104,6 +110,51 @@ struct brcm_nand_dma_desc {
 #define NAND_CTRL_RDY			(INTFC_CTLR_READY | INTFC_FLASH_READY)
 #define NAND_POLL_STATUS_TIMEOUT_MS	100
 
+/* flash_dma registers */
+enum flash_dma_reg {
+	FLASH_DMA_REVISION = 0,
+	FLASH_DMA_FIRST_DESC,
+	FLASH_DMA_FIRST_DESC_EXT,
+	FLASH_DMA_CTRL,
+	FLASH_DMA_MODE,
+	FLASH_DMA_STATUS,
+	FLASH_DMA_INTERRUPT_DESC,
+	FLASH_DMA_INTERRUPT_DESC_EXT,
+	FLASH_DMA_ERROR_STATUS,
+	FLASH_DMA_CURRENT_DESC,
+	FLASH_DMA_CURRENT_DESC_EXT,
+};
+
+/* flash_dma registers v1*/
+static const u16 flash_dma_regs_v1[] = {
+	[FLASH_DMA_REVISION]		= 0x00,
+	[FLASH_DMA_FIRST_DESC]		= 0x04,
+	[FLASH_DMA_FIRST_DESC_EXT]	= 0x08,
+	[FLASH_DMA_CTRL]		= 0x0c,
+	[FLASH_DMA_MODE]		= 0x10,
+	[FLASH_DMA_STATUS]		= 0x14,
+	[FLASH_DMA_INTERRUPT_DESC]	= 0x18,
+	[FLASH_DMA_INTERRUPT_DESC_EXT]	= 0x1c,
+	[FLASH_DMA_ERROR_STATUS]	= 0x20,
+	[FLASH_DMA_CURRENT_DESC]	= 0x24,
+	[FLASH_DMA_CURRENT_DESC_EXT]	= 0x28,
+};
+
+/* flash_dma registers v4 */
+static const u16 flash_dma_regs_v4[] = {
+	[FLASH_DMA_REVISION]		= 0x00,
+	[FLASH_DMA_FIRST_DESC]		= 0x08,
+	[FLASH_DMA_FIRST_DESC_EXT]	= 0x0c,
+	[FLASH_DMA_CTRL]		= 0x10,
+	[FLASH_DMA_MODE]		= 0x14,
+	[FLASH_DMA_STATUS]		= 0x18,
+	[FLASH_DMA_INTERRUPT_DESC]	= 0x20,
+	[FLASH_DMA_INTERRUPT_DESC_EXT]	= 0x24,
+	[FLASH_DMA_ERROR_STATUS]	= 0x28,
+	[FLASH_DMA_CURRENT_DESC]	= 0x30,
+	[FLASH_DMA_CURRENT_DESC_EXT]	= 0x34,
+};
+
 /* Controller feature flags */
 enum {
 	BRCMNAND_HAS_1K_SECTORS			= BIT(0),
@@ -136,6 +187,8 @@ struct brcmnand_controller {
 	/* List of NAND hosts (one for each chip-select) */
 	struct list_head host_list;
 
+	/* flash_dma reg */
+	const u16		*flash_dma_offsets;
 	struct brcm_nand_dma_desc *dma_desc;
 	dma_addr_t		dma_pa;
 
@@ -470,7 +523,7 @@ static int brcmnand_revision_init(struct brcmnand_controller *ctrl)
 	/* Register offsets */
 	if (ctrl->nand_version >= 0x0702)
 		ctrl->reg_offsets = brcmnand_regs_v72;
-	else if (ctrl->nand_version >= 0x0701)
+	else if (ctrl->nand_version == 0x0701)
 		ctrl->reg_offsets = brcmnand_regs_v71;
 	else if (ctrl->nand_version >= 0x0600)
 		ctrl->reg_offsets = brcmnand_regs_v60;
@@ -515,7 +568,7 @@ static int brcmnand_revision_init(struct brcmnand_controller *ctrl)
 	}
 
 	/* Maximum spare area sector size (per 512B) */
-	if (ctrl->nand_version >= 0x0702)
+	if (ctrl->nand_version == 0x0702)
 		ctrl->max_oob = 128;
 	else if (ctrl->nand_version >= 0x0600)
 		ctrl->max_oob = 64;
@@ -546,6 +599,15 @@ static int brcmnand_revision_init(struct brcmnand_controller *ctrl)
 	return 0;
 }
 
+static void brcmnand_flash_dma_revision_init(struct brcmnand_controller *ctrl)
+{
+	/* flash_dma register offsets */
+	if (ctrl->nand_version >= 0x0703)
+		ctrl->flash_dma_offsets = flash_dma_regs_v4;
+	else
+		ctrl->flash_dma_offsets = flash_dma_regs_v1;
+}
+
 static inline u32 brcmnand_read_reg(struct brcmnand_controller *ctrl,
 		enum brcmnand_reg reg)
 {
@@ -668,7 +730,7 @@ static void brcmnand_wr_corr_thresh(struct brcmnand_host *host, u8 val)
 	enum brcmnand_reg reg = BRCMNAND_CORR_THRESHOLD;
 	int cs = host->cs;
 
-	if (ctrl->nand_version >= 0x0702)
+	if (ctrl->nand_version == 0x0702)
 		bits = 7;
 	else if (ctrl->nand_version >= 0x0600)
 		bits = 6;
@@ -722,7 +784,7 @@ enum {
 
 static inline u32 brcmnand_spare_area_mask(struct brcmnand_controller *ctrl)
 {
-	if (ctrl->nand_version >= 0x0702)
+	if (ctrl->nand_version == 0x0702)
 		return GENMASK(7, 0);
 	else if (ctrl->nand_version >= 0x0600)
 		return GENMASK(6, 0);
@@ -852,20 +914,6 @@ static inline void brcmnand_set_wp(struct brcmnand_controller *ctrl, bool en)
  * Flash DMA
  ***********************************************************************/
 
-enum flash_dma_reg {
-	FLASH_DMA_REVISION		= 0x00,
-	FLASH_DMA_FIRST_DESC		= 0x04,
-	FLASH_DMA_FIRST_DESC_EXT	= 0x08,
-	FLASH_DMA_CTRL			= 0x0c,
-	FLASH_DMA_MODE			= 0x10,
-	FLASH_DMA_STATUS		= 0x14,
-	FLASH_DMA_INTERRUPT_DESC	= 0x18,
-	FLASH_DMA_INTERRUPT_DESC_EXT	= 0x1c,
-	FLASH_DMA_ERROR_STATUS		= 0x20,
-	FLASH_DMA_CURRENT_DESC		= 0x24,
-	FLASH_DMA_CURRENT_DESC_EXT	= 0x28,
-};
-
 static inline bool has_flash_dma(struct brcmnand_controller *ctrl)
 {
 	return ctrl->flash_dma_base;
@@ -877,14 +925,19 @@ static inline bool flash_dma_buf_ok(const void *buf)
 		likely(IS_ALIGNED((uintptr_t)buf, 4));
 }
 
-static inline void flash_dma_writel(struct brcmnand_controller *ctrl, u8 offs,
-				    u32 val)
+static inline void flash_dma_writel(struct brcmnand_controller *ctrl,
+				    enum flash_dma_reg dma_reg, u32 val)
 {
+	u16 offs = ctrl->flash_dma_offsets[dma_reg];
+
 	brcmnand_writel(val, ctrl->flash_dma_base + offs);
 }
 
-static inline u32 flash_dma_readl(struct brcmnand_controller *ctrl, u8 offs)
+static inline u32 flash_dma_readl(struct brcmnand_controller *ctrl,
+				  enum flash_dma_reg dma_reg)
 {
+	u16 offs = ctrl->flash_dma_offsets[dma_reg];
+
 	return brcmnand_readl(ctrl->flash_dma_base + offs);
 }
 
@@ -2427,6 +2480,7 @@ static int brcmnand_resume(struct device *dev)
 	{ .compatible = "brcm,brcmnand-v7.0" },
 	{ .compatible = "brcm,brcmnand-v7.1" },
 	{ .compatible = "brcm,brcmnand-v7.2" },
+	{ .compatible = "brcm,brcmnand-v7.3" },
 	{},
 };
 MODULE_DEVICE_TABLE(of, brcmnand_of_match);
@@ -2513,7 +2567,11 @@ int brcmnand_probe(struct platform_device *pdev, struct brcmnand_soc *soc)
 			goto err;
 		}
 
-		flash_dma_writel(ctrl, FLASH_DMA_MODE, 1); /* linked-list */
+		/* initialize the dma version */
+		brcmnand_flash_dma_revision_init(ctrl);
+
+		/* linked-list and stop on error */
+		flash_dma_writel(ctrl, FLASH_DMA_MODE, FLASH_DMA_MODE_MASK);
 		flash_dma_writel(ctrl, FLASH_DMA_ERROR_STATUS, 0);
 
 		/* Allocate descriptor(s) */
-- 
1.9.0.138.g2de3478


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

^ permalink raw reply related	[flat|nested] 14+ messages in thread
* [PATCH 3/3] dt: bindings: mtd: brcmand: Add brcmnand,brcmnand-v7.3 support
  2019-05-30 21:20 ` Kamal Dasu
@ 2019-05-30 21:20   ` Kamal Dasu
  -1 siblings, 0 replies; 14+ messages in thread
From: Kamal Dasu @ 2019-05-30 21:20 UTC (permalink / raw)
  To: linux-mtd
  Cc: bcm-kernel-feedback-list, linux-kernel, Kamal Dasu,
	David Woodhouse, Brian Norris, Marek Vasut, Miquel Raynal,
	Richard Weinberger, Vignesh Raghavendra, Rob Herring,
	Mark Rutland, devicetree

Added brcm,brcmnand-v7.3 as possible compatible string to support
brcmnand controller v7.3.

Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
---
 Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt
index 0b7c373..ad4cd30 100644
--- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt
+++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt
@@ -28,6 +28,7 @@ Required properties:
                          brcm,brcmnand-v7.0
                          brcm,brcmnand-v7.1
                          brcm,brcmnand-v7.2
+                         brcm,brcmnand-v7.3
                          brcm,brcmnand
 - reg              : the register start and length for NAND register region.
                      (optional) Flash DMA register range (if present)
-- 
1.9.0.138.g2de3478


^ permalink raw reply related	[flat|nested] 14+ messages in thread
* [PATCH 3/3] dt: bindings: mtd: brcmand: Add brcmnand, brcmnand-v7.3 support
@ 2019-05-30 21:20   ` Kamal Dasu
  0 siblings, 0 replies; 14+ messages in thread
From: Kamal Dasu @ 2019-05-30 21:20 UTC (permalink / raw)
  To: linux-mtd
  Cc: Mark Rutland, devicetree, Vignesh Raghavendra, Kamal Dasu,
	Richard Weinberger, linux-kernel, Marek Vasut, Rob Herring,
	bcm-kernel-feedback-list, Miquel Raynal, Brian Norris,
	David Woodhouse

Added brcm,brcmnand-v7.3 as possible compatible string to support
brcmnand controller v7.3.

Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
---
 Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt
index 0b7c373..ad4cd30 100644
--- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt
+++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt
@@ -28,6 +28,7 @@ Required properties:
                          brcm,brcmnand-v7.0
                          brcm,brcmnand-v7.1
                          brcm,brcmnand-v7.2
+                         brcm,brcmnand-v7.3
                          brcm,brcmnand
 - reg              : the register start and length for NAND register region.
                      (optional) Flash DMA register range (if present)
-- 
1.9.0.138.g2de3478


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

^ permalink raw reply related	[flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] mtd: nand: raw: brcmnand: Refactored code and introduced inline functions
  2019-05-30 21:20 ` Kamal Dasu
@ 2019-06-01  7:57   ` Boris Brezillon
  -1 siblings, 0 replies; 14+ messages in thread
From: Boris Brezillon @ 2019-06-01  7:57 UTC (permalink / raw)
  To: Kamal Dasu
  Cc: linux-mtd, Vignesh Raghavendra, Richard Weinberger, linux-kernel,
	Marek Vasut, bcm-kernel-feedback-list, Miquel Raynal,
	Brian Norris, David Woodhouse

On Thu, 30 May 2019 17:20:35 -0400
Kamal Dasu <kdasu.kdev@gmail.com> wrote:

> Refactored NAND ECC and CMD address configuration code to use inline
> functions.

I'd expect the compiler to be smart enough to decide when inlining is
appropriate. Did you check that adding the inline specifier actually
makes a difference?

> 
> Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
> ---
>  drivers/mtd/nand/raw/brcmnand/brcmnand.c | 100 +++++++++++++++++++------------
>  1 file changed, 62 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> index ce0b8ff..77b7850 100644
> --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> @@ -588,6 +588,54 @@ static inline void brcmnand_write_fc(struct brcmnand_controller *ctrl,
>  	__raw_writel(val, ctrl->nand_fc + word * 4);
>  }
>  
> +static inline void brcmnand_clear_ecc_addr(struct brcmnand_controller *ctrl)
> +{
> +
> +	/* Clear error addresses */
> +	brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_ADDR, 0);
> +	brcmnand_write_reg(ctrl, BRCMNAND_CORR_ADDR, 0);
> +	brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_EXT_ADDR, 0);
> +	brcmnand_write_reg(ctrl, BRCMNAND_CORR_EXT_ADDR, 0);
> +}
> +
> +static inline u64 brcmnand_get_uncorrecc_addr(struct brcmnand_controller *ctrl)
> +{
> +	u64 err_addr;
> +
> +	err_addr = brcmnand_read_reg(ctrl, BRCMNAND_UNCORR_ADDR);
> +	err_addr |= ((u64)(brcmnand_read_reg(ctrl,
> +					     BRCMNAND_UNCORR_EXT_ADDR)
> +					     & 0xffff) << 32);
> +
> +	return err_addr;
> +}
> +
> +static inline u64 brcmnand_get_correcc_addr(struct brcmnand_controller *ctrl)
> +{
> +	u64 err_addr;
> +
> +	err_addr = brcmnand_read_reg(ctrl, BRCMNAND_CORR_ADDR);
> +	err_addr |= ((u64)(brcmnand_read_reg(ctrl,
> +					     BRCMNAND_CORR_EXT_ADDR)
> +					     & 0xffff) << 32);
> +
> +	return err_addr;
> +}
> +
> +static inline void brcmnand_set_cmd_addr(struct mtd_info *mtd, u64 addr)
> +{
> +	struct nand_chip *chip =  mtd_to_nand(mtd);
> +	struct brcmnand_host *host = nand_get_controller_data(chip);
> +	struct brcmnand_controller *ctrl = host->ctrl;
> +
> +	brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS,
> +			   (host->cs << 16) | ((addr >> 32) & 0xffff));
> +	(void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS);
> +	brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS,
> +			   lower_32_bits(addr));
> +	(void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS);
> +}
> +
>  static inline u16 brcmnand_cs_offset(struct brcmnand_controller *ctrl, int cs,
>  				     enum brcmnand_cs_reg reg)
>  {
> @@ -1213,9 +1261,12 @@ static void brcmnand_send_cmd(struct brcmnand_host *host, int cmd)
>  {
>  	struct brcmnand_controller *ctrl = host->ctrl;
>  	int ret;
> +	u64 cmd_addr;
> +
> +	cmd_addr = brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS);
> +
> +	dev_dbg(ctrl->dev, "send native cmd %d addr 0x%llx\n", cmd, cmd_addr);
>  
> -	dev_dbg(ctrl->dev, "send native cmd %d addr_lo 0x%x\n", cmd,
> -		brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS));
>  	BUG_ON(ctrl->cmd_pending != 0);
>  	ctrl->cmd_pending = cmd;
>  
> @@ -1374,12 +1425,7 @@ static void brcmnand_cmdfunc(struct nand_chip *chip, unsigned command,
>  	if (!native_cmd)
>  		return;
>  
> -	brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS,
> -		(host->cs << 16) | ((addr >> 32) & 0xffff));
> -	(void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS);
> -	brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS, lower_32_bits(addr));
> -	(void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS);
> -
> +	brcmnand_set_cmd_addr(mtd, addr);
>  	brcmnand_send_cmd(host, native_cmd);
>  	brcmnand_waitfunc(chip);
>  
> @@ -1597,20 +1643,10 @@ static int brcmnand_read_by_pio(struct mtd_info *mtd, struct nand_chip *chip,
>  	struct brcmnand_controller *ctrl = host->ctrl;
>  	int i, j, ret = 0;
>  
> -	/* Clear error addresses */
> -	brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_ADDR, 0);
> -	brcmnand_write_reg(ctrl, BRCMNAND_CORR_ADDR, 0);
> -	brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_EXT_ADDR, 0);
> -	brcmnand_write_reg(ctrl, BRCMNAND_CORR_EXT_ADDR, 0);
> -
> -	brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS,
> -			(host->cs << 16) | ((addr >> 32) & 0xffff));
> -	(void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS);
> +	brcmnand_clear_ecc_addr(ctrl);
>  
>  	for (i = 0; i < trans; i++, addr += FC_BYTES) {
> -		brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS,
> -				   lower_32_bits(addr));
> -		(void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS);
> +		brcmnand_set_cmd_addr(mtd, addr);
>  		/* SPARE_AREA_READ does not use ECC, so just use PAGE_READ */
>  		brcmnand_send_cmd(host, CMD_PAGE_READ);
>  		brcmnand_waitfunc(chip);
> @@ -1630,21 +1666,15 @@ static int brcmnand_read_by_pio(struct mtd_info *mtd, struct nand_chip *chip,
>  					host->hwcfg.sector_size_1k);
>  
>  		if (!ret) {
> -			*err_addr = brcmnand_read_reg(ctrl,
> -					BRCMNAND_UNCORR_ADDR) |
> -				((u64)(brcmnand_read_reg(ctrl,
> -						BRCMNAND_UNCORR_EXT_ADDR)
> -					& 0xffff) << 32);
> +			*err_addr = brcmnand_get_uncorrecc_addr(ctrl);
> +
>  			if (*err_addr)
>  				ret = -EBADMSG;
>  		}
>  
>  		if (!ret) {
> -			*err_addr = brcmnand_read_reg(ctrl,
> -					BRCMNAND_CORR_ADDR) |
> -				((u64)(brcmnand_read_reg(ctrl,
> -						BRCMNAND_CORR_EXT_ADDR)
> -					& 0xffff) << 32);
> +			*err_addr = brcmnand_get_correcc_addr(ctrl);
> +
>  			if (*err_addr)
>  				ret = -EUCLEAN;
>  		}
> @@ -1711,7 +1741,7 @@ static int brcmnand_read(struct mtd_info *mtd, struct nand_chip *chip,
>  	dev_dbg(ctrl->dev, "read %llx -> %p\n", (unsigned long long)addr, buf);
>  
>  try_dmaread:
> -	brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_COUNT, 0);
> +	brcmnand_clear_ecc_addr(ctrl);
>  
>  	if (has_flash_dma(ctrl) && !oob && flash_dma_buf_ok(buf)) {
>  		err = brcmnand_dma_trans(host, addr, buf, trans * FC_BYTES,
> @@ -1858,15 +1888,9 @@ static int brcmnand_write(struct mtd_info *mtd, struct nand_chip *chip,
>  		goto out;
>  	}
>  
> -	brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS,
> -			(host->cs << 16) | ((addr >> 32) & 0xffff));
> -	(void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS);
> -
>  	for (i = 0; i < trans; i++, addr += FC_BYTES) {
>  		/* full address MUST be set before populating FC */
> -		brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS,
> -				   lower_32_bits(addr));
> -		(void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS);
> +		brcmnand_set_cmd_addr(mtd, addr);
>  
>  		if (buf) {
>  			brcmnand_soc_data_bus_prepare(ctrl->soc, false);


^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] mtd: nand: raw: brcmnand: Refactored code and introduced inline functions
@ 2019-06-01  7:57   ` Boris Brezillon
  0 siblings, 0 replies; 14+ messages in thread
From: Boris Brezillon @ 2019-06-01  7:57 UTC (permalink / raw)
  To: Kamal Dasu
  Cc: Vignesh Raghavendra, Richard Weinberger, linux-kernel,
	Marek Vasut, bcm-kernel-feedback-list, Miquel Raynal, linux-mtd,
	Brian Norris, David Woodhouse

On Thu, 30 May 2019 17:20:35 -0400
Kamal Dasu <kdasu.kdev@gmail.com> wrote:

> Refactored NAND ECC and CMD address configuration code to use inline
> functions.

I'd expect the compiler to be smart enough to decide when inlining is
appropriate. Did you check that adding the inline specifier actually
makes a difference?

> 
> Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
> ---
>  drivers/mtd/nand/raw/brcmnand/brcmnand.c | 100 +++++++++++++++++++------------
>  1 file changed, 62 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> index ce0b8ff..77b7850 100644
> --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> @@ -588,6 +588,54 @@ static inline void brcmnand_write_fc(struct brcmnand_controller *ctrl,
>  	__raw_writel(val, ctrl->nand_fc + word * 4);
>  }
>  
> +static inline void brcmnand_clear_ecc_addr(struct brcmnand_controller *ctrl)
> +{
> +
> +	/* Clear error addresses */
> +	brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_ADDR, 0);
> +	brcmnand_write_reg(ctrl, BRCMNAND_CORR_ADDR, 0);
> +	brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_EXT_ADDR, 0);
> +	brcmnand_write_reg(ctrl, BRCMNAND_CORR_EXT_ADDR, 0);
> +}
> +
> +static inline u64 brcmnand_get_uncorrecc_addr(struct brcmnand_controller *ctrl)
> +{
> +	u64 err_addr;
> +
> +	err_addr = brcmnand_read_reg(ctrl, BRCMNAND_UNCORR_ADDR);
> +	err_addr |= ((u64)(brcmnand_read_reg(ctrl,
> +					     BRCMNAND_UNCORR_EXT_ADDR)
> +					     & 0xffff) << 32);
> +
> +	return err_addr;
> +}
> +
> +static inline u64 brcmnand_get_correcc_addr(struct brcmnand_controller *ctrl)
> +{
> +	u64 err_addr;
> +
> +	err_addr = brcmnand_read_reg(ctrl, BRCMNAND_CORR_ADDR);
> +	err_addr |= ((u64)(brcmnand_read_reg(ctrl,
> +					     BRCMNAND_CORR_EXT_ADDR)
> +					     & 0xffff) << 32);
> +
> +	return err_addr;
> +}
> +
> +static inline void brcmnand_set_cmd_addr(struct mtd_info *mtd, u64 addr)
> +{
> +	struct nand_chip *chip =  mtd_to_nand(mtd);
> +	struct brcmnand_host *host = nand_get_controller_data(chip);
> +	struct brcmnand_controller *ctrl = host->ctrl;
> +
> +	brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS,
> +			   (host->cs << 16) | ((addr >> 32) & 0xffff));
> +	(void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS);
> +	brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS,
> +			   lower_32_bits(addr));
> +	(void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS);
> +}
> +
>  static inline u16 brcmnand_cs_offset(struct brcmnand_controller *ctrl, int cs,
>  				     enum brcmnand_cs_reg reg)
>  {
> @@ -1213,9 +1261,12 @@ static void brcmnand_send_cmd(struct brcmnand_host *host, int cmd)
>  {
>  	struct brcmnand_controller *ctrl = host->ctrl;
>  	int ret;
> +	u64 cmd_addr;
> +
> +	cmd_addr = brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS);
> +
> +	dev_dbg(ctrl->dev, "send native cmd %d addr 0x%llx\n", cmd, cmd_addr);
>  
> -	dev_dbg(ctrl->dev, "send native cmd %d addr_lo 0x%x\n", cmd,
> -		brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS));
>  	BUG_ON(ctrl->cmd_pending != 0);
>  	ctrl->cmd_pending = cmd;
>  
> @@ -1374,12 +1425,7 @@ static void brcmnand_cmdfunc(struct nand_chip *chip, unsigned command,
>  	if (!native_cmd)
>  		return;
>  
> -	brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS,
> -		(host->cs << 16) | ((addr >> 32) & 0xffff));
> -	(void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS);
> -	brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS, lower_32_bits(addr));
> -	(void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS);
> -
> +	brcmnand_set_cmd_addr(mtd, addr);
>  	brcmnand_send_cmd(host, native_cmd);
>  	brcmnand_waitfunc(chip);
>  
> @@ -1597,20 +1643,10 @@ static int brcmnand_read_by_pio(struct mtd_info *mtd, struct nand_chip *chip,
>  	struct brcmnand_controller *ctrl = host->ctrl;
>  	int i, j, ret = 0;
>  
> -	/* Clear error addresses */
> -	brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_ADDR, 0);
> -	brcmnand_write_reg(ctrl, BRCMNAND_CORR_ADDR, 0);
> -	brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_EXT_ADDR, 0);
> -	brcmnand_write_reg(ctrl, BRCMNAND_CORR_EXT_ADDR, 0);
> -
> -	brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS,
> -			(host->cs << 16) | ((addr >> 32) & 0xffff));
> -	(void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS);
> +	brcmnand_clear_ecc_addr(ctrl);
>  
>  	for (i = 0; i < trans; i++, addr += FC_BYTES) {
> -		brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS,
> -				   lower_32_bits(addr));
> -		(void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS);
> +		brcmnand_set_cmd_addr(mtd, addr);
>  		/* SPARE_AREA_READ does not use ECC, so just use PAGE_READ */
>  		brcmnand_send_cmd(host, CMD_PAGE_READ);
>  		brcmnand_waitfunc(chip);
> @@ -1630,21 +1666,15 @@ static int brcmnand_read_by_pio(struct mtd_info *mtd, struct nand_chip *chip,
>  					host->hwcfg.sector_size_1k);
>  
>  		if (!ret) {
> -			*err_addr = brcmnand_read_reg(ctrl,
> -					BRCMNAND_UNCORR_ADDR) |
> -				((u64)(brcmnand_read_reg(ctrl,
> -						BRCMNAND_UNCORR_EXT_ADDR)
> -					& 0xffff) << 32);
> +			*err_addr = brcmnand_get_uncorrecc_addr(ctrl);
> +
>  			if (*err_addr)
>  				ret = -EBADMSG;
>  		}
>  
>  		if (!ret) {
> -			*err_addr = brcmnand_read_reg(ctrl,
> -					BRCMNAND_CORR_ADDR) |
> -				((u64)(brcmnand_read_reg(ctrl,
> -						BRCMNAND_CORR_EXT_ADDR)
> -					& 0xffff) << 32);
> +			*err_addr = brcmnand_get_correcc_addr(ctrl);
> +
>  			if (*err_addr)
>  				ret = -EUCLEAN;
>  		}
> @@ -1711,7 +1741,7 @@ static int brcmnand_read(struct mtd_info *mtd, struct nand_chip *chip,
>  	dev_dbg(ctrl->dev, "read %llx -> %p\n", (unsigned long long)addr, buf);
>  
>  try_dmaread:
> -	brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_COUNT, 0);
> +	brcmnand_clear_ecc_addr(ctrl);
>  
>  	if (has_flash_dma(ctrl) && !oob && flash_dma_buf_ok(buf)) {
>  		err = brcmnand_dma_trans(host, addr, buf, trans * FC_BYTES,
> @@ -1858,15 +1888,9 @@ static int brcmnand_write(struct mtd_info *mtd, struct nand_chip *chip,
>  		goto out;
>  	}
>  
> -	brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS,
> -			(host->cs << 16) | ((addr >> 32) & 0xffff));
> -	(void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS);
> -
>  	for (i = 0; i < trans; i++, addr += FC_BYTES) {
>  		/* full address MUST be set before populating FC */
> -		brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS,
> -				   lower_32_bits(addr));
> -		(void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS);
> +		brcmnand_set_cmd_addr(mtd, addr);
>  
>  		if (buf) {
>  			brcmnand_soc_data_bus_prepare(ctrl->soc, false);


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

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] mtd: nand: raw: brcmnand: Refactored code and introduced inline functions
  2019-06-01  7:57   ` Boris Brezillon
@ 2019-06-03 14:11     ` Kamal Dasu
  -1 siblings, 0 replies; 14+ messages in thread
From: Kamal Dasu @ 2019-06-03 14:11 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: MTD Maling List, Vignesh Raghavendra, Richard Weinberger,
	Linux Kernel Mailing List, Marek Vasut, bcm-kernel-feedback-list,
	Miquel Raynal, Brian Norris, David Woodhouse

Boris,

On Sat, Jun 1, 2019 at 3:57 AM Boris Brezillon
<boris.brezillon@collabora.com> wrote:
>
> On Thu, 30 May 2019 17:20:35 -0400
> Kamal Dasu <kdasu.kdev@gmail.com> wrote:
>
> > Refactored NAND ECC and CMD address configuration code to use inline
> > functions.
>
> I'd expect the compiler to be smart enough to decide when inlining is
> appropriate. Did you check that adding the inline specifier actually
> makes a difference?

This was done to make the code more readable. It does not make any
difference to performance.

>
> >
> > Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
> > ---
> >  drivers/mtd/nand/raw/brcmnand/brcmnand.c | 100 +++++++++++++++++++------------
> >  1 file changed, 62 insertions(+), 38 deletions(-)
> >
> > diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> > index ce0b8ff..77b7850 100644
> > --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> > +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> > @@ -588,6 +588,54 @@ static inline void brcmnand_write_fc(struct brcmnand_controller *ctrl,
> >       __raw_writel(val, ctrl->nand_fc + word * 4);
> >  }
> >
> > +static inline void brcmnand_clear_ecc_addr(struct brcmnand_controller *ctrl)
> > +{
> > +
> > +     /* Clear error addresses */
> > +     brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_ADDR, 0);
> > +     brcmnand_write_reg(ctrl, BRCMNAND_CORR_ADDR, 0);
> > +     brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_EXT_ADDR, 0);
> > +     brcmnand_write_reg(ctrl, BRCMNAND_CORR_EXT_ADDR, 0);
> > +}
> > +
> > +static inline u64 brcmnand_get_uncorrecc_addr(struct brcmnand_controller *ctrl)
> > +{
> > +     u64 err_addr;
> > +
> > +     err_addr = brcmnand_read_reg(ctrl, BRCMNAND_UNCORR_ADDR);
> > +     err_addr |= ((u64)(brcmnand_read_reg(ctrl,
> > +                                          BRCMNAND_UNCORR_EXT_ADDR)
> > +                                          & 0xffff) << 32);
> > +
> > +     return err_addr;
> > +}
> > +
> > +static inline u64 brcmnand_get_correcc_addr(struct brcmnand_controller *ctrl)
> > +{
> > +     u64 err_addr;
> > +
> > +     err_addr = brcmnand_read_reg(ctrl, BRCMNAND_CORR_ADDR);
> > +     err_addr |= ((u64)(brcmnand_read_reg(ctrl,
> > +                                          BRCMNAND_CORR_EXT_ADDR)
> > +                                          & 0xffff) << 32);
> > +
> > +     return err_addr;
> > +}
> > +
> > +static inline void brcmnand_set_cmd_addr(struct mtd_info *mtd, u64 addr)
> > +{
> > +     struct nand_chip *chip =  mtd_to_nand(mtd);
> > +     struct brcmnand_host *host = nand_get_controller_data(chip);
> > +     struct brcmnand_controller *ctrl = host->ctrl;
> > +
> > +     brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS,
> > +                        (host->cs << 16) | ((addr >> 32) & 0xffff));
> > +     (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS);
> > +     brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS,
> > +                        lower_32_bits(addr));
> > +     (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS);
> > +}
> > +
> >  static inline u16 brcmnand_cs_offset(struct brcmnand_controller *ctrl, int cs,
> >                                    enum brcmnand_cs_reg reg)
> >  {
> > @@ -1213,9 +1261,12 @@ static void brcmnand_send_cmd(struct brcmnand_host *host, int cmd)
> >  {
> >       struct brcmnand_controller *ctrl = host->ctrl;
> >       int ret;
> > +     u64 cmd_addr;
> > +
> > +     cmd_addr = brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS);
> > +
> > +     dev_dbg(ctrl->dev, "send native cmd %d addr 0x%llx\n", cmd, cmd_addr);
> >
> > -     dev_dbg(ctrl->dev, "send native cmd %d addr_lo 0x%x\n", cmd,
> > -             brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS));
> >       BUG_ON(ctrl->cmd_pending != 0);
> >       ctrl->cmd_pending = cmd;
> >
> > @@ -1374,12 +1425,7 @@ static void brcmnand_cmdfunc(struct nand_chip *chip, unsigned command,
> >       if (!native_cmd)
> >               return;
> >
> > -     brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS,
> > -             (host->cs << 16) | ((addr >> 32) & 0xffff));
> > -     (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS);
> > -     brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS, lower_32_bits(addr));
> > -     (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS);
> > -
> > +     brcmnand_set_cmd_addr(mtd, addr);
> >       brcmnand_send_cmd(host, native_cmd);
> >       brcmnand_waitfunc(chip);
> >
> > @@ -1597,20 +1643,10 @@ static int brcmnand_read_by_pio(struct mtd_info *mtd, struct nand_chip *chip,
> >       struct brcmnand_controller *ctrl = host->ctrl;
> >       int i, j, ret = 0;
> >
> > -     /* Clear error addresses */
> > -     brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_ADDR, 0);
> > -     brcmnand_write_reg(ctrl, BRCMNAND_CORR_ADDR, 0);
> > -     brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_EXT_ADDR, 0);
> > -     brcmnand_write_reg(ctrl, BRCMNAND_CORR_EXT_ADDR, 0);
> > -
> > -     brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS,
> > -                     (host->cs << 16) | ((addr >> 32) & 0xffff));
> > -     (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS);
> > +     brcmnand_clear_ecc_addr(ctrl);
> >
> >       for (i = 0; i < trans; i++, addr += FC_BYTES) {
> > -             brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS,
> > -                                lower_32_bits(addr));
> > -             (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS);
> > +             brcmnand_set_cmd_addr(mtd, addr);
> >               /* SPARE_AREA_READ does not use ECC, so just use PAGE_READ */
> >               brcmnand_send_cmd(host, CMD_PAGE_READ);
> >               brcmnand_waitfunc(chip);
> > @@ -1630,21 +1666,15 @@ static int brcmnand_read_by_pio(struct mtd_info *mtd, struct nand_chip *chip,
> >                                       host->hwcfg.sector_size_1k);
> >
> >               if (!ret) {
> > -                     *err_addr = brcmnand_read_reg(ctrl,
> > -                                     BRCMNAND_UNCORR_ADDR) |
> > -                             ((u64)(brcmnand_read_reg(ctrl,
> > -                                             BRCMNAND_UNCORR_EXT_ADDR)
> > -                                     & 0xffff) << 32);
> > +                     *err_addr = brcmnand_get_uncorrecc_addr(ctrl);
> > +
> >                       if (*err_addr)
> >                               ret = -EBADMSG;
> >               }
> >
> >               if (!ret) {
> > -                     *err_addr = brcmnand_read_reg(ctrl,
> > -                                     BRCMNAND_CORR_ADDR) |
> > -                             ((u64)(brcmnand_read_reg(ctrl,
> > -                                             BRCMNAND_CORR_EXT_ADDR)
> > -                                     & 0xffff) << 32);
> > +                     *err_addr = brcmnand_get_correcc_addr(ctrl);
> > +
> >                       if (*err_addr)
> >                               ret = -EUCLEAN;
> >               }
> > @@ -1711,7 +1741,7 @@ static int brcmnand_read(struct mtd_info *mtd, struct nand_chip *chip,
> >       dev_dbg(ctrl->dev, "read %llx -> %p\n", (unsigned long long)addr, buf);
> >
> >  try_dmaread:
> > -     brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_COUNT, 0);
> > +     brcmnand_clear_ecc_addr(ctrl);
> >
> >       if (has_flash_dma(ctrl) && !oob && flash_dma_buf_ok(buf)) {
> >               err = brcmnand_dma_trans(host, addr, buf, trans * FC_BYTES,
> > @@ -1858,15 +1888,9 @@ static int brcmnand_write(struct mtd_info *mtd, struct nand_chip *chip,
> >               goto out;
> >       }
> >
> > -     brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS,
> > -                     (host->cs << 16) | ((addr >> 32) & 0xffff));
> > -     (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS);
> > -
> >       for (i = 0; i < trans; i++, addr += FC_BYTES) {
> >               /* full address MUST be set before populating FC */
> > -             brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS,
> > -                                lower_32_bits(addr));
> > -             (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS);
> > +             brcmnand_set_cmd_addr(mtd, addr);
> >
> >               if (buf) {
> >                       brcmnand_soc_data_bus_prepare(ctrl->soc, false);
>

Thanks
Kamal

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] mtd: nand: raw: brcmnand: Refactored code and introduced inline functions
@ 2019-06-03 14:11     ` Kamal Dasu
  0 siblings, 0 replies; 14+ messages in thread
From: Kamal Dasu @ 2019-06-03 14:11 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Vignesh Raghavendra, Richard Weinberger,
	Linux Kernel Mailing List, Marek Vasut, bcm-kernel-feedback-list,
	Miquel Raynal, MTD Maling List, Brian Norris, David Woodhouse

Boris,

On Sat, Jun 1, 2019 at 3:57 AM Boris Brezillon
<boris.brezillon@collabora.com> wrote:
>
> On Thu, 30 May 2019 17:20:35 -0400
> Kamal Dasu <kdasu.kdev@gmail.com> wrote:
>
> > Refactored NAND ECC and CMD address configuration code to use inline
> > functions.
>
> I'd expect the compiler to be smart enough to decide when inlining is
> appropriate. Did you check that adding the inline specifier actually
> makes a difference?

This was done to make the code more readable. It does not make any
difference to performance.

>
> >
> > Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
> > ---
> >  drivers/mtd/nand/raw/brcmnand/brcmnand.c | 100 +++++++++++++++++++------------
> >  1 file changed, 62 insertions(+), 38 deletions(-)
> >
> > diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> > index ce0b8ff..77b7850 100644
> > --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> > +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> > @@ -588,6 +588,54 @@ static inline void brcmnand_write_fc(struct brcmnand_controller *ctrl,
> >       __raw_writel(val, ctrl->nand_fc + word * 4);
> >  }
> >
> > +static inline void brcmnand_clear_ecc_addr(struct brcmnand_controller *ctrl)
> > +{
> > +
> > +     /* Clear error addresses */
> > +     brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_ADDR, 0);
> > +     brcmnand_write_reg(ctrl, BRCMNAND_CORR_ADDR, 0);
> > +     brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_EXT_ADDR, 0);
> > +     brcmnand_write_reg(ctrl, BRCMNAND_CORR_EXT_ADDR, 0);
> > +}
> > +
> > +static inline u64 brcmnand_get_uncorrecc_addr(struct brcmnand_controller *ctrl)
> > +{
> > +     u64 err_addr;
> > +
> > +     err_addr = brcmnand_read_reg(ctrl, BRCMNAND_UNCORR_ADDR);
> > +     err_addr |= ((u64)(brcmnand_read_reg(ctrl,
> > +                                          BRCMNAND_UNCORR_EXT_ADDR)
> > +                                          & 0xffff) << 32);
> > +
> > +     return err_addr;
> > +}
> > +
> > +static inline u64 brcmnand_get_correcc_addr(struct brcmnand_controller *ctrl)
> > +{
> > +     u64 err_addr;
> > +
> > +     err_addr = brcmnand_read_reg(ctrl, BRCMNAND_CORR_ADDR);
> > +     err_addr |= ((u64)(brcmnand_read_reg(ctrl,
> > +                                          BRCMNAND_CORR_EXT_ADDR)
> > +                                          & 0xffff) << 32);
> > +
> > +     return err_addr;
> > +}
> > +
> > +static inline void brcmnand_set_cmd_addr(struct mtd_info *mtd, u64 addr)
> > +{
> > +     struct nand_chip *chip =  mtd_to_nand(mtd);
> > +     struct brcmnand_host *host = nand_get_controller_data(chip);
> > +     struct brcmnand_controller *ctrl = host->ctrl;
> > +
> > +     brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS,
> > +                        (host->cs << 16) | ((addr >> 32) & 0xffff));
> > +     (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS);
> > +     brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS,
> > +                        lower_32_bits(addr));
> > +     (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS);
> > +}
> > +
> >  static inline u16 brcmnand_cs_offset(struct brcmnand_controller *ctrl, int cs,
> >                                    enum brcmnand_cs_reg reg)
> >  {
> > @@ -1213,9 +1261,12 @@ static void brcmnand_send_cmd(struct brcmnand_host *host, int cmd)
> >  {
> >       struct brcmnand_controller *ctrl = host->ctrl;
> >       int ret;
> > +     u64 cmd_addr;
> > +
> > +     cmd_addr = brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS);
> > +
> > +     dev_dbg(ctrl->dev, "send native cmd %d addr 0x%llx\n", cmd, cmd_addr);
> >
> > -     dev_dbg(ctrl->dev, "send native cmd %d addr_lo 0x%x\n", cmd,
> > -             brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS));
> >       BUG_ON(ctrl->cmd_pending != 0);
> >       ctrl->cmd_pending = cmd;
> >
> > @@ -1374,12 +1425,7 @@ static void brcmnand_cmdfunc(struct nand_chip *chip, unsigned command,
> >       if (!native_cmd)
> >               return;
> >
> > -     brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS,
> > -             (host->cs << 16) | ((addr >> 32) & 0xffff));
> > -     (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS);
> > -     brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS, lower_32_bits(addr));
> > -     (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS);
> > -
> > +     brcmnand_set_cmd_addr(mtd, addr);
> >       brcmnand_send_cmd(host, native_cmd);
> >       brcmnand_waitfunc(chip);
> >
> > @@ -1597,20 +1643,10 @@ static int brcmnand_read_by_pio(struct mtd_info *mtd, struct nand_chip *chip,
> >       struct brcmnand_controller *ctrl = host->ctrl;
> >       int i, j, ret = 0;
> >
> > -     /* Clear error addresses */
> > -     brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_ADDR, 0);
> > -     brcmnand_write_reg(ctrl, BRCMNAND_CORR_ADDR, 0);
> > -     brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_EXT_ADDR, 0);
> > -     brcmnand_write_reg(ctrl, BRCMNAND_CORR_EXT_ADDR, 0);
> > -
> > -     brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS,
> > -                     (host->cs << 16) | ((addr >> 32) & 0xffff));
> > -     (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS);
> > +     brcmnand_clear_ecc_addr(ctrl);
> >
> >       for (i = 0; i < trans; i++, addr += FC_BYTES) {
> > -             brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS,
> > -                                lower_32_bits(addr));
> > -             (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS);
> > +             brcmnand_set_cmd_addr(mtd, addr);
> >               /* SPARE_AREA_READ does not use ECC, so just use PAGE_READ */
> >               brcmnand_send_cmd(host, CMD_PAGE_READ);
> >               brcmnand_waitfunc(chip);
> > @@ -1630,21 +1666,15 @@ static int brcmnand_read_by_pio(struct mtd_info *mtd, struct nand_chip *chip,
> >                                       host->hwcfg.sector_size_1k);
> >
> >               if (!ret) {
> > -                     *err_addr = brcmnand_read_reg(ctrl,
> > -                                     BRCMNAND_UNCORR_ADDR) |
> > -                             ((u64)(brcmnand_read_reg(ctrl,
> > -                                             BRCMNAND_UNCORR_EXT_ADDR)
> > -                                     & 0xffff) << 32);
> > +                     *err_addr = brcmnand_get_uncorrecc_addr(ctrl);
> > +
> >                       if (*err_addr)
> >                               ret = -EBADMSG;
> >               }
> >
> >               if (!ret) {
> > -                     *err_addr = brcmnand_read_reg(ctrl,
> > -                                     BRCMNAND_CORR_ADDR) |
> > -                             ((u64)(brcmnand_read_reg(ctrl,
> > -                                             BRCMNAND_CORR_EXT_ADDR)
> > -                                     & 0xffff) << 32);
> > +                     *err_addr = brcmnand_get_correcc_addr(ctrl);
> > +
> >                       if (*err_addr)
> >                               ret = -EUCLEAN;
> >               }
> > @@ -1711,7 +1741,7 @@ static int brcmnand_read(struct mtd_info *mtd, struct nand_chip *chip,
> >       dev_dbg(ctrl->dev, "read %llx -> %p\n", (unsigned long long)addr, buf);
> >
> >  try_dmaread:
> > -     brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_COUNT, 0);
> > +     brcmnand_clear_ecc_addr(ctrl);
> >
> >       if (has_flash_dma(ctrl) && !oob && flash_dma_buf_ok(buf)) {
> >               err = brcmnand_dma_trans(host, addr, buf, trans * FC_BYTES,
> > @@ -1858,15 +1888,9 @@ static int brcmnand_write(struct mtd_info *mtd, struct nand_chip *chip,
> >               goto out;
> >       }
> >
> > -     brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS,
> > -                     (host->cs << 16) | ((addr >> 32) & 0xffff));
> > -     (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS);
> > -
> >       for (i = 0; i < trans; i++, addr += FC_BYTES) {
> >               /* full address MUST be set before populating FC */
> > -             brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS,
> > -                                lower_32_bits(addr));
> > -             (void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS);
> > +             brcmnand_set_cmd_addr(mtd, addr);
> >
> >               if (buf) {
> >                       brcmnand_soc_data_bus_prepare(ctrl->soc, false);
>

Thanks
Kamal

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

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] mtd: nand: raw: brcmnand: Refactored code and introduced inline functions
  2019-06-03 14:11     ` Kamal Dasu
@ 2019-06-03 14:18       ` Boris Brezillon
  -1 siblings, 0 replies; 14+ messages in thread
From: Boris Brezillon @ 2019-06-03 14:18 UTC (permalink / raw)
  To: Kamal Dasu
  Cc: MTD Maling List, Vignesh Raghavendra, Richard Weinberger,
	Linux Kernel Mailing List, Marek Vasut, bcm-kernel-feedback-list,
	Miquel Raynal, Brian Norris, David Woodhouse

On Mon, 3 Jun 2019 10:11:20 -0400
Kamal Dasu <kdasu.kdev@gmail.com> wrote:

> Boris,
> 
> On Sat, Jun 1, 2019 at 3:57 AM Boris Brezillon
> <boris.brezillon@collabora.com> wrote:
> >
> > On Thu, 30 May 2019 17:20:35 -0400
> > Kamal Dasu <kdasu.kdev@gmail.com> wrote:
> >  
> > > Refactored NAND ECC and CMD address configuration code to use inline
> > > functions.  
> >
> > I'd expect the compiler to be smart enough to decide when inlining is
> > appropriate. Did you check that adding the inline specifier actually
> > makes a difference?  
> 
> This was done to make the code more readable. It does not make any
> difference to performance.

I meant dropping the inline specifier, not going back to manual
inlining. As a general rule, you don't need to add the 'inline'
specifier unless your function is defined in a header. In all other
cases the compiler is able to inline things on its own when it sees the
number of instructions is small enough or when the function is only
called once.

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] mtd: nand: raw: brcmnand: Refactored code and introduced inline functions
@ 2019-06-03 14:18       ` Boris Brezillon
  0 siblings, 0 replies; 14+ messages in thread
From: Boris Brezillon @ 2019-06-03 14:18 UTC (permalink / raw)
  To: Kamal Dasu
  Cc: Vignesh Raghavendra, Richard Weinberger,
	Linux Kernel Mailing List, Marek Vasut, bcm-kernel-feedback-list,
	Miquel Raynal, MTD Maling List, Brian Norris, David Woodhouse

On Mon, 3 Jun 2019 10:11:20 -0400
Kamal Dasu <kdasu.kdev@gmail.com> wrote:

> Boris,
> 
> On Sat, Jun 1, 2019 at 3:57 AM Boris Brezillon
> <boris.brezillon@collabora.com> wrote:
> >
> > On Thu, 30 May 2019 17:20:35 -0400
> > Kamal Dasu <kdasu.kdev@gmail.com> wrote:
> >  
> > > Refactored NAND ECC and CMD address configuration code to use inline
> > > functions.  
> >
> > I'd expect the compiler to be smart enough to decide when inlining is
> > appropriate. Did you check that adding the inline specifier actually
> > makes a difference?  
> 
> This was done to make the code more readable. It does not make any
> difference to performance.

I meant dropping the inline specifier, not going back to manual
inlining. As a general rule, you don't need to add the 'inline'
specifier unless your function is defined in a header. In all other
cases the compiler is able to inline things on its own when it sees the
number of instructions is small enough or when the function is only
called once.

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

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] mtd: nand: raw: brcmnand: Refactored code and introduced inline functions
  2019-06-03 14:18       ` Boris Brezillon
@ 2019-06-03 14:39         ` Kamal Dasu
  -1 siblings, 0 replies; 14+ messages in thread
From: Kamal Dasu @ 2019-06-03 14:39 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: MTD Maling List, Vignesh Raghavendra, Richard Weinberger,
	Linux Kernel Mailing List, Marek Vasut, bcm-kernel-feedback-list,
	Miquel Raynal, Brian Norris, David Woodhouse

On Mon, Jun 3, 2019 at 10:18 AM Boris Brezillon
<boris.brezillon@collabora.com> wrote:
>
> On Mon, 3 Jun 2019 10:11:20 -0400
> Kamal Dasu <kdasu.kdev@gmail.com> wrote:
>
> > Boris,
> >
> > On Sat, Jun 1, 2019 at 3:57 AM Boris Brezillon
> > <boris.brezillon@collabora.com> wrote:
> > >
> > > On Thu, 30 May 2019 17:20:35 -0400
> > > Kamal Dasu <kdasu.kdev@gmail.com> wrote:
> > >
> > > > Refactored NAND ECC and CMD address configuration code to use inline
> > > > functions.
> > >
> > > I'd expect the compiler to be smart enough to decide when inlining is
> > > appropriate. Did you check that adding the inline specifier actually
> > > makes a difference?
> >
> > This was done to make the code more readable. It does not make any
> > difference to performance.
>
> I meant dropping the inline specifier, not going back to manual
> inlining. As a general rule, you don't need to add the 'inline'
> specifier unless your function is defined in a header. In all other
> cases the compiler is able to inline things on its own when it sees the
> number of instructions is small enough or when the function is only
> called once.

Oh ok got it, will get rid if the inline specifier  and send a v2
version of the change.

Thanks
Kamal

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] mtd: nand: raw: brcmnand: Refactored code and introduced inline functions
@ 2019-06-03 14:39         ` Kamal Dasu
  0 siblings, 0 replies; 14+ messages in thread
From: Kamal Dasu @ 2019-06-03 14:39 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Vignesh Raghavendra, Richard Weinberger,
	Linux Kernel Mailing List, Marek Vasut, bcm-kernel-feedback-list,
	Miquel Raynal, MTD Maling List, Brian Norris, David Woodhouse

On Mon, Jun 3, 2019 at 10:18 AM Boris Brezillon
<boris.brezillon@collabora.com> wrote:
>
> On Mon, 3 Jun 2019 10:11:20 -0400
> Kamal Dasu <kdasu.kdev@gmail.com> wrote:
>
> > Boris,
> >
> > On Sat, Jun 1, 2019 at 3:57 AM Boris Brezillon
> > <boris.brezillon@collabora.com> wrote:
> > >
> > > On Thu, 30 May 2019 17:20:35 -0400
> > > Kamal Dasu <kdasu.kdev@gmail.com> wrote:
> > >
> > > > Refactored NAND ECC and CMD address configuration code to use inline
> > > > functions.
> > >
> > > I'd expect the compiler to be smart enough to decide when inlining is
> > > appropriate. Did you check that adding the inline specifier actually
> > > makes a difference?
> >
> > This was done to make the code more readable. It does not make any
> > difference to performance.
>
> I meant dropping the inline specifier, not going back to manual
> inlining. As a general rule, you don't need to add the 'inline'
> specifier unless your function is defined in a header. In all other
> cases the compiler is able to inline things on its own when it sees the
> number of instructions is small enough or when the function is only
> called once.

Oh ok got it, will get rid if the inline specifier  and send a v2
version of the change.

Thanks
Kamal

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

^ permalink raw reply	[flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-06-03 14:39 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-30 21:20 [PATCH 1/3] mtd: nand: raw: brcmnand: Refactored code and introduced inline functions Kamal Dasu
2019-05-30 21:20 ` Kamal Dasu
2019-05-30 21:20 ` [PATCH 2/3] mtd: nand: raw: brcmnand: Add support for v7.3 controller Kamal Dasu
2019-05-30 21:20   ` Kamal Dasu
2019-05-30 21:20 ` [PATCH 3/3] dt: bindings: mtd: brcmand: Add brcmnand,brcmnand-v7.3 support Kamal Dasu
2019-05-30 21:20   ` [PATCH 3/3] dt: bindings: mtd: brcmand: Add brcmnand, brcmnand-v7.3 support Kamal Dasu
2019-06-01  7:57 ` [PATCH 1/3] mtd: nand: raw: brcmnand: Refactored code and introduced inline functions Boris Brezillon
2019-06-01  7:57   ` Boris Brezillon
2019-06-03 14:11   ` Kamal Dasu
2019-06-03 14:11     ` Kamal Dasu
2019-06-03 14:18     ` Boris Brezillon
2019-06-03 14:18       ` Boris Brezillon
2019-06-03 14:39       ` Kamal Dasu
2019-06-03 14:39         ` Kamal Dasu

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