[PATCH 0/7] mtd: brcmnand: add support for NAND core on bcma bus

[PATCH 0/7] mtd: brcmnand: add support for NAND core on bcma bus

[Hauke Mehrtens]

These patches are adding support for the NAND controller on the BCM53xx 
and BCM47xx arm SoCs (Northstar). These SoCs are using the axi bus 
driven by bcma and not only device tree.

This was tested on a Netgear R6250 with a BCM4708 SoC.
The patches are based on top of current l2-mtd/master tree.

Hauke Mehrtens (7):
  mtd: brcmnand: remove double new line from print
  mtd: brcmnand: do not make local variable static
  mtd: brcmnand: use struct device and not platform_device
  mtd: brcmnand: add methods to register struct device
  mtd: brcmnand: add bcma driver
  mtd: brcmnand: run bcm47xxpart part parser in addition
  ARM: BCM5301X: add NAND flash chip description

 arch/arm/boot/dts/bcm4708-luxul-xwc-1000.dts |  12 +--
 arch/arm/boot/dts/bcm5301x.dtsi              |  19 ++++
 drivers/mtd/nand/Kconfig                     |   8 ++
 drivers/mtd/nand/brcmnand/Makefile           |   1 +
 drivers/mtd/nand/brcmnand/bcm63138_nand.c    |   2 +-
 drivers/mtd/nand/brcmnand/bcma_nand.c        | 153 +++++++++++++++++++++++++++
 drivers/mtd/nand/brcmnand/brcmnand.c         | 150 ++++++++++++++++----------
 drivers/mtd/nand/brcmnand/brcmnand.h         |  10 +-
 drivers/mtd/nand/brcmnand/iproc_nand.c       |   2 +-
 9 files changed, 293 insertions(+), 64 deletions(-)
 create mode 100644 drivers/mtd/nand/brcmnand/bcma_nand.c

-- 
2.1.4

[PATCH 1/7] mtd: brcmnand: remove double new line from print

[Hauke Mehrtens]

The caller already adds a new line and in the other cases there is no
new line added.

Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 drivers/mtd/nand/brcmnand/brcmnand.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/brcmnand/brcmnand.c b/drivers/mtd/nand/brcmnand/brcmnand.c
index a780768..bae30ab 100644
--- a/drivers/mtd/nand/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/brcmnand/brcmnand.c
@@ -1765,7 +1765,7 @@ static void brcmnand_print_cfg(char *buf, struct brcmnand_cfg *cfg)
 	else if (cfg->sector_size_1k)
 		sprintf(buf, ", BCH-%u (1KiB sector)", cfg->ecc_level << 1);
 	else
-		sprintf(buf, ", BCH-%u\n", cfg->ecc_level);
+		sprintf(buf, ", BCH-%u", cfg->ecc_level);
 }
 
 /*
-- 
2.1.4

Re: [PATCH 1/7] mtd: brcmnand: remove double new line from print

[Brian Norris]

On Sun, May 17, 2015 at 05:41:00PM +0200, Hauke Mehrtens wrote:
> The caller already adds a new line and in the other cases there is no
> new line added.
> 
> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
> ---
>  drivers/mtd/nand/brcmnand/brcmnand.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/brcmnand/brcmnand.c b/drivers/mtd/nand/brcmnand/brcmnand.c
> index a780768..bae30ab 100644
> --- a/drivers/mtd/nand/brcmnand/brcmnand.c
> +++ b/drivers/mtd/nand/brcmnand/brcmnand.c
> @@ -1765,7 +1765,7 @@ static void brcmnand_print_cfg(char *buf, struct brcmnand_cfg *cfg)
>  	else if (cfg->sector_size_1k)
>  		sprintf(buf, ", BCH-%u (1KiB sector)", cfg->ecc_level << 1);
>  	else
> -		sprintf(buf, ", BCH-%u\n", cfg->ecc_level);
> +		sprintf(buf, ", BCH-%u", cfg->ecc_level);
>  }
>  
>  /*

Good catch. Applied to l2-mtd.git.

[PATCH 2/7] mtd: brcmnand: do not make local variable static

[Hauke Mehrtens]

Remove static in front of ctrl. This variable should not be shared
between different instances of brcmnand_probe(), it should be local to
this function and stored on the stack.

Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 drivers/mtd/nand/brcmnand/brcmnand.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/brcmnand/brcmnand.c b/drivers/mtd/nand/brcmnand/brcmnand.c
index bae30ab..a48ad49 100644
--- a/drivers/mtd/nand/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/brcmnand/brcmnand.c
@@ -2069,7 +2069,7 @@ int brcmnand_probe(struct platform_device *pdev, struct brcmnand_soc *soc)
 {
 	struct device *dev = &pdev->dev;
 	struct device_node *dn = dev->of_node, *child;
-	static struct brcmnand_controller *ctrl;
+	struct brcmnand_controller *ctrl;
 	struct resource *res;
 	int ret;
 
-- 
2.1.4

Re: [PATCH 2/7] mtd: brcmnand: do not make local variable static

[Brian Norris]

On Sun, May 17, 2015 at 05:41:01PM +0200, Hauke Mehrtens wrote:
> Remove static in front of ctrl. This variable should not be shared
> between different instances of brcmnand_probe(), it should be local to
> this function and stored on the stack.
> 
> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
> ---
>  drivers/mtd/nand/brcmnand/brcmnand.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/brcmnand/brcmnand.c b/drivers/mtd/nand/brcmnand/brcmnand.c
> index bae30ab..a48ad49 100644
> --- a/drivers/mtd/nand/brcmnand/brcmnand.c
> +++ b/drivers/mtd/nand/brcmnand/brcmnand.c
> @@ -2069,7 +2069,7 @@ int brcmnand_probe(struct platform_device *pdev, struct brcmnand_soc *soc)
>  {
>  	struct device *dev = &pdev->dev;
>  	struct device_node *dn = dev->of_node, *child;
> -	static struct brcmnand_controller *ctrl;
> +	struct brcmnand_controller *ctrl;
>  	struct resource *res;
>  	int ret;
>  

Good catch. Applied this to l2-mtd.git. I'll come back to the others.

[PATCH 3/7] mtd: brcmnand: use struct device and not platform_device

[Hauke Mehrtens]

This replaces the struct platform_device with struct device in the
structures used by brcmnand. This is a first step to make it possible
to use other device types than platform devices.

This should not change any functionality.

Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 drivers/mtd/nand/brcmnand/bcm63138_nand.c |  2 +-
 drivers/mtd/nand/brcmnand/brcmnand.c      | 20 +++++++++-----------
 drivers/mtd/nand/brcmnand/brcmnand.h      |  2 +-
 drivers/mtd/nand/brcmnand/iproc_nand.c    |  2 +-
 4 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/mtd/nand/brcmnand/bcm63138_nand.c b/drivers/mtd/nand/brcmnand/bcm63138_nand.c
index 3f4c44c..d764807 100644
--- a/drivers/mtd/nand/brcmnand/bcm63138_nand.c
+++ b/drivers/mtd/nand/brcmnand/bcm63138_nand.c
@@ -81,7 +81,7 @@ static int bcm63138_nand_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->base))
 		return PTR_ERR(priv->base);
 
-	soc->pdev = pdev;
+	soc->dev = dev;
 	soc->priv = priv;
 	soc->ctlrdy_ack = bcm63138_nand_intc_ack;
 	soc->ctlrdy_set_enabled = bcm63138_nand_intc_set;
diff --git a/drivers/mtd/nand/brcmnand/brcmnand.c b/drivers/mtd/nand/brcmnand/brcmnand.c
index a48ad49..58462b5 100644
--- a/drivers/mtd/nand/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/brcmnand/brcmnand.c
@@ -180,7 +180,7 @@ struct brcmnand_host {
 
 	struct nand_chip	chip;
 	struct mtd_info		mtd;
-	struct platform_device	*pdev;
+	struct device		*dev;
 	int			cs;
 
 	unsigned int		last_cmd;
@@ -739,7 +739,7 @@ static struct nand_ecclayout *brcmnand_create_layout(int ecc_level,
 	int sas;
 	int idx1, idx2;
 
-	layout = devm_kzalloc(&host->pdev->dev, sizeof(*layout), GFP_KERNEL);
+	layout = devm_kzalloc(host->dev, sizeof(*layout), GFP_KERNEL);
 	if (!layout)
 		return NULL;
 
@@ -783,7 +783,7 @@ static struct nand_ecclayout *brcmnand_create_layout(int ecc_level,
 	 */
 	req = DIV_ROUND_UP(ecc_level * 14, 8);
 	if (req >= sas) {
-		dev_err(&host->pdev->dev,
+		dev_err(host->dev,
 			"error: ECC too large for OOB (ECC bytes %d, spare sector %d)\n",
 			req, sas);
 		return NULL;
@@ -842,8 +842,7 @@ static struct nand_ecclayout *brcmstb_choose_ecc_layout(
 
 	layout = brcmnand_create_layout(ecc_level, host);
 	if (!layout) {
-		dev_err(&host->pdev->dev,
-				"no proper ecc_layout for this NAND cfg\n");
+		dev_err(host->dev, "no proper ecc_layout for this NAND cfg\n");
 		return NULL;
 	}
 
@@ -1884,7 +1883,7 @@ static int brcmnand_init_cs(struct brcmnand_host *host)
 {
 	struct brcmnand_controller *ctrl = host->ctrl;
 	struct device_node *dn = host->of_node;
-	struct platform_device *pdev = host->pdev;
+	struct device *dev = host->dev;
 	struct mtd_info *mtd;
 	struct nand_chip *chip;
 	int ret = 0;
@@ -1892,7 +1891,7 @@ static int brcmnand_init_cs(struct brcmnand_host *host)
 
 	ret = of_property_read_u32(dn, "reg", &host->cs);
 	if (ret) {
-		dev_err(&pdev->dev, "can't get chip-select\n");
+		dev_err(dev, "can't get chip-select\n");
 		return -ENXIO;
 	}
 
@@ -1902,10 +1901,9 @@ static int brcmnand_init_cs(struct brcmnand_host *host)
 	chip->dn = dn;
 	chip->priv = host;
 	mtd->priv = chip;
-	mtd->name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "brcmnand.%d",
-				   host->cs);
+	mtd->name = devm_kasprintf(dev, GFP_KERNEL, "brcmnand.%d", host->cs);
 	mtd->owner = THIS_MODULE;
-	mtd->dev.parent = &pdev->dev;
+	mtd->dev.parent = dev;
 
 	chip->IO_ADDR_R = (void __iomem *)0xdeadbeef;
 	chip->IO_ADDR_W = (void __iomem *)0xdeadbeef;
@@ -2205,7 +2203,7 @@ int brcmnand_probe(struct platform_device *pdev, struct brcmnand_soc *soc)
 			host = devm_kzalloc(dev, sizeof(*host), GFP_KERNEL);
 			if (!host)
 				return -ENOMEM;
-			host->pdev = pdev;
+			host->dev = dev;
 			host->ctrl = ctrl;
 			host->of_node = child;
 
diff --git a/drivers/mtd/nand/brcmnand/brcmnand.h b/drivers/mtd/nand/brcmnand/brcmnand.h
index a20c736..d0d74d7 100644
--- a/drivers/mtd/nand/brcmnand/brcmnand.h
+++ b/drivers/mtd/nand/brcmnand/brcmnand.h
@@ -21,7 +21,7 @@ struct platform_device;
 struct dev_pm_ops;
 
 struct brcmnand_soc {
-	struct platform_device *pdev;
+	struct device *dev;
 	void *priv;
 	bool (*ctlrdy_ack)(struct brcmnand_soc *soc);
 	void (*ctlrdy_set_enabled)(struct brcmnand_soc *soc, bool en);
diff --git a/drivers/mtd/nand/brcmnand/iproc_nand.c b/drivers/mtd/nand/brcmnand/iproc_nand.c
index 683495c..50504e9 100644
--- a/drivers/mtd/nand/brcmnand/iproc_nand.c
+++ b/drivers/mtd/nand/brcmnand/iproc_nand.c
@@ -118,7 +118,7 @@ static int iproc_nand_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->ext_base))
 		return PTR_ERR(priv->ext_base);
 
-	soc->pdev = pdev;
+	soc->dev = dev;
 	soc->priv = priv;
 	soc->ctlrdy_ack = iproc_nand_intc_ack;
 	soc->ctlrdy_set_enabled = iproc_nand_intc_set;
-- 
2.1.4

[PATCH 4/7] mtd: brcmnand: add methods to register struct device

[Hauke Mehrtens]

This commit adds a new method to register a struct device to brcmnand.
The functions handling platform device will stay, but they are just
extracting the data from the platform device and are providing the data
to the brcmnand_probe_dev() function.

The struct brcmnand_soc is now used to provide all information like irq
numbers and memory ranges to the actual driver. soc is now set in all
cases.

Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 drivers/mtd/nand/brcmnand/brcmnand.c | 121 +++++++++++++++++++++++------------
 drivers/mtd/nand/brcmnand/brcmnand.h |   8 +++
 2 files changed, 89 insertions(+), 40 deletions(-)

diff --git a/drivers/mtd/nand/brcmnand/brcmnand.c b/drivers/mtd/nand/brcmnand/brcmnand.c
index 58462b5..468a212 100644
--- a/drivers/mtd/nand/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/brcmnand/brcmnand.c
@@ -2023,7 +2023,7 @@ static int brcmnand_resume(struct device *dev)
 	brcmnand_write_reg(ctrl, BRCMNAND_CS_XOR, ctrl->nand_cs_nand_xor);
 	brcmnand_write_reg(ctrl, BRCMNAND_CORR_THRESHOLD,
 			ctrl->corr_stat_threshold);
-	if (ctrl->soc) {
+	if (ctrl->soc->ctlrdy_ack && ctrl->soc->ctlrdy_set_enabled) {
 		/* Clear/re-enable interrupt */
 		ctrl->soc->ctlrdy_ack(ctrl->soc);
 		ctrl->soc->ctlrdy_set_enabled(ctrl->soc, true);
@@ -2063,21 +2063,12 @@ MODULE_DEVICE_TABLE(of, brcmnand_of_match);
  * Platform driver setup (per controller)
  ***********************************************************************/
 
-int brcmnand_probe(struct platform_device *pdev, struct brcmnand_soc *soc)
+int brcmnand_probe_dev(struct device *dev, struct brcmnand_soc *soc)
 {
-	struct device *dev = &pdev->dev;
 	struct device_node *dn = dev->of_node, *child;
 	struct brcmnand_controller *ctrl;
-	struct resource *res;
 	int ret;
 
-	/* We only support device-tree instantiation */
-	if (!dn)
-		return -ENODEV;
-
-	if (!of_match_node(brcmnand_of_match, dn))
-		return -ENODEV;
-
 	ctrl = devm_kzalloc(dev, sizeof(*ctrl), GFP_KERNEL);
 	if (!ctrl)
 		return -ENOMEM;
@@ -2092,10 +2083,7 @@ int brcmnand_probe(struct platform_device *pdev, struct brcmnand_soc *soc)
 	INIT_LIST_HEAD(&ctrl->host_list);
 
 	/* NAND register range */
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	ctrl->nand_base = devm_ioremap_resource(dev, res);
-	if (IS_ERR(ctrl->nand_base))
-		return PTR_ERR(ctrl->nand_base);
+	ctrl->nand_base = soc->nand_base;
 
 	/* Initialize NAND revision */
 	ret = brcmnand_revision_init(ctrl);
@@ -2106,22 +2094,16 @@ int brcmnand_probe(struct platform_device *pdev, struct brcmnand_soc *soc)
 	 * Most chips have this cache at a fixed offset within 'nand' block.
 	 * Some must specify this region separately.
 	 */
-	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "nand-cache");
-	if (res) {
-		ctrl->nand_fc = devm_ioremap_resource(dev, res);
-		if (IS_ERR(ctrl->nand_fc))
-			return PTR_ERR(ctrl->nand_fc);
+	if (soc->nand_fc) {
+		ctrl->nand_fc = soc->nand_fc;
 	} else {
 		ctrl->nand_fc = ctrl->nand_base +
 				ctrl->reg_offsets[BRCMNAND_FC_BASE];
 	}
 
 	/* FLASH_DMA */
-	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "flash-dma");
-	if (res) {
-		ctrl->flash_dma_base = devm_ioremap_resource(dev, res);
-		if (IS_ERR(ctrl->flash_dma_base))
-			return PTR_ERR(ctrl->flash_dma_base);
+	if (soc->flash_dma_base) {
+		ctrl->flash_dma_base = soc->flash_dma_base;
 
 		flash_dma_writel(ctrl, FLASH_DMA_MODE, 1); /* linked-list */
 		flash_dma_writel(ctrl, FLASH_DMA_ERROR_STATUS, 0);
@@ -2133,11 +2115,7 @@ int brcmnand_probe(struct platform_device *pdev, struct brcmnand_soc *soc)
 		if (!ctrl->dma_desc)
 			return -ENOMEM;
 
-		ctrl->dma_irq = platform_get_irq(pdev, 1);
-		if ((int)ctrl->dma_irq < 0) {
-			dev_err(dev, "missing FLASH_DMA IRQ\n");
-			return -ENODEV;
-		}
+		ctrl->dma_irq = soc->dma_irq;
 
 		ret = devm_request_irq(dev, ctrl->dma_irq,
 				brcmnand_dma_irq, 0, DRV_NAME,
@@ -2166,17 +2144,13 @@ int brcmnand_probe(struct platform_device *pdev, struct brcmnand_soc *soc)
 	}
 
 	/* IRQ */
-	ctrl->irq = platform_get_irq(pdev, 0);
-	if ((int)ctrl->irq < 0) {
-		dev_err(dev, "no IRQ defined\n");
-		return -ENODEV;
-	}
+	ctrl->irq = soc->irq;
 
 	/*
 	 * Some SoCs integrate this controller (e.g., its interrupt bits) in
 	 * interesting ways
 	 */
-	if (soc) {
+	if (soc->ctlrdy_ack && soc->ctlrdy_set_enabled) {
 		ctrl->soc = soc;
 
 		ret = devm_request_irq(dev, ctrl->irq, brcmnand_irq, 0,
@@ -2221,20 +2195,87 @@ int brcmnand_probe(struct platform_device *pdev, struct brcmnand_soc *soc)
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(brcmnand_probe);
+EXPORT_SYMBOL_GPL(brcmnand_probe_dev);
 
-int brcmnand_remove(struct platform_device *pdev)
+int brcmnand_remove_dev(struct device *dev)
 {
-	struct brcmnand_controller *ctrl = dev_get_drvdata(&pdev->dev);
+	struct brcmnand_controller *ctrl = dev_get_drvdata(dev);
 	struct brcmnand_host *host;
 
 	list_for_each_entry(host, &ctrl->host_list, node)
 		nand_release(&host->mtd);
 
-	dev_set_drvdata(&pdev->dev, NULL);
+	dev_set_drvdata(dev, NULL);
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(brcmnand_remove_dev);
+
+int brcmnand_probe(struct platform_device *pdev, struct brcmnand_soc *soc)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *dn = dev->of_node;
+	struct resource *res;
+
+	/* We only support device-tree instantiation */
+	if (!dn)
+		return -ENODEV;
+
+	if (!of_match_node(brcmnand_of_match, dn))
+		return -ENODEV;
+
+	if (!soc) {
+		soc = devm_kzalloc(dev, sizeof(*soc), GFP_KERNEL);
+		if (!soc)
+			return -ENOMEM;
+	}
+
+	/* NAND register range */
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	soc->nand_base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(soc->nand_base))
+		return PTR_ERR(soc->nand_base);
+
+	/*
+	 * Most chips have this cache at a fixed offset within 'nand' block.
+	 * Some must specify this region separately.
+	 */
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "nand-cache");
+	if (res) {
+		soc->nand_fc = devm_ioremap_resource(dev, res);
+		if (IS_ERR(soc->nand_fc))
+			return PTR_ERR(soc->nand_fc);
+	}
+
+	/* FLASH_DMA */
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "flash-dma");
+	if (res) {
+		soc->flash_dma_base = devm_ioremap_resource(dev, res);
+		if (IS_ERR(soc->flash_dma_base))
+			return PTR_ERR(soc->flash_dma_base);
+
+		soc->dma_irq = platform_get_irq(pdev, 1);
+		if ((int)soc->dma_irq < 0) {
+			dev_err(dev, "missing FLASH_DMA IRQ\n");
+			return -ENODEV;
+		}
+	}
+
+	/* IRQ */
+	soc->irq = platform_get_irq(pdev, 0);
+	if ((int)soc->irq < 0) {
+		dev_err(dev, "no IRQ defined\n");
+		return -ENODEV;
+	}
+
+	return brcmnand_probe_dev(&pdev->dev, soc);
+}
+EXPORT_SYMBOL_GPL(brcmnand_probe);
+
+int brcmnand_remove(struct platform_device *pdev)
+{
+	return brcmnand_remove_dev(&pdev->dev);
+}
 EXPORT_SYMBOL_GPL(brcmnand_remove);
 
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/mtd/nand/brcmnand/brcmnand.h b/drivers/mtd/nand/brcmnand/brcmnand.h
index d0d74d7..3211401 100644
--- a/drivers/mtd/nand/brcmnand/brcmnand.h
+++ b/drivers/mtd/nand/brcmnand/brcmnand.h
@@ -26,6 +26,11 @@ struct brcmnand_soc {
 	bool (*ctlrdy_ack)(struct brcmnand_soc *soc);
 	void (*ctlrdy_set_enabled)(struct brcmnand_soc *soc, bool en);
 	void (*prepare_data_bus)(struct brcmnand_soc *soc, bool prepare);
+	void __iomem	*nand_base;
+	void __iomem	*nand_fc; /* flash cache */
+	void __iomem	*flash_dma_base;
+	unsigned int	irq;
+	unsigned int	dma_irq;
 };
 
 static inline void brcmnand_soc_data_bus_prepare(struct brcmnand_soc *soc)
@@ -65,6 +70,9 @@ static inline void brcmnand_writel(u32 val, void __iomem *addr)
 		writel_relaxed(val, addr);
 }
 
+int brcmnand_probe_dev(struct device *dev, struct brcmnand_soc *soc);
+int brcmnand_remove_dev(struct device *dev);
+
 int brcmnand_probe(struct platform_device *pdev, struct brcmnand_soc *soc);
 int brcmnand_remove(struct platform_device *pdev);
 
-- 
2.1.4

[PATCH 5/7] mtd: brcmnand: add bcma driver

[Hauke Mehrtens]

This driver registers at the bcma bus and drives the NAND core if it
was found on this bus. The bcma bus with this NAND core is used on the
bcm53xx and bcm47xx Northstar SoC with ARM CPU cores. The memory ranges
are automatically detected by bcma and the irq numbers read from device
tree by bcma bus driver.

This is based on the iproc driver.

Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 drivers/mtd/nand/Kconfig              |   8 ++
 drivers/mtd/nand/brcmnand/Makefile    |   1 +
 drivers/mtd/nand/brcmnand/bcma_nand.c | 153 ++++++++++++++++++++++++++++++++++
 3 files changed, 162 insertions(+)
 create mode 100644 drivers/mtd/nand/brcmnand/bcma_nand.c

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 376b538..b698212 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -402,6 +402,14 @@ config MTD_NAND_BRCMNAND
 	  originally designed for Set-Top Box but is used on various BCM7xxx,
 	  BCM3xxx, BCM63xxx, iProc/Cygnus and more.
 
+config MTD_NAND_BRCMNAND_BCMA
+	tristate "Broadcom BCMA NAND controller"
+	depends on ARM || MIPS
+	select MTD_NAND_BRCMNAND
+	help
+	  Enables the driver which registers Broadcom NAND controller against
+	  bcma.
+
 config MTD_NAND_BCM47XXNFLASH
 	tristate "Support for NAND flash on BCM4706 BCMA bus"
 	depends on BCMA_NFLASH
diff --git a/drivers/mtd/nand/brcmnand/Makefile b/drivers/mtd/nand/brcmnand/Makefile
index 3b1fbfd..8483984 100644
--- a/drivers/mtd/nand/brcmnand/Makefile
+++ b/drivers/mtd/nand/brcmnand/Makefile
@@ -1,6 +1,7 @@
 # link order matters; don't link the more generic brcmstb_nand.o before the
 # more specific iproc_nand.o, for instance
 obj-$(CONFIG_MTD_NAND_BRCMNAND)		+= iproc_nand.o
+obj-$(CONFIG_MTD_NAND_BRCMNAND_BCMA)	+= bcma_nand.o
 obj-$(CONFIG_MTD_NAND_BRCMNAND)		+= bcm63138_nand.o
 obj-$(CONFIG_MTD_NAND_BRCMNAND)		+= brcmstb_nand.o
 obj-$(CONFIG_MTD_NAND_BRCMNAND)		+= brcmnand.o
diff --git a/drivers/mtd/nand/brcmnand/bcma_nand.c b/drivers/mtd/nand/brcmnand/bcma_nand.c
new file mode 100644
index 0000000..58380bd
--- /dev/null
+++ b/drivers/mtd/nand/brcmnand/bcma_nand.c
@@ -0,0 +1,153 @@
+/*
+ * Copyright © 2015 Broadcom Corporation
+ * Copyright 2015 Hauke Mehrtens <hauke@hauke-m.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/ioport.h>
+#include <linux/module.h>
+#include <linux/bcma/bcma.h>
+#include <linux/slab.h>
+
+#include "brcmnand.h"
+
+struct bcma_nand_soc_priv {
+	struct brcmnand_soc soc;
+	struct bcma_device *core;
+	spinlock_t idm_lock;
+};
+
+#define BCMA_NAND_CTLR_READY_OFFSET       0x0f10
+#define BCMA_NAND_CTLR_READY              BIT(0)
+
+#define BCMA_NAND_APB_LE_MODE             BIT(24)
+#define BCMA_NAND_INT_CTRL_READ_ENABLE    BIT(6)
+
+static bool bcma_nand_intc_ack(struct brcmnand_soc *soc)
+{
+	struct bcma_nand_soc_priv *priv = soc->priv;
+	struct bcma_device *core = priv->core;
+
+	u32 val = bcma_read32(core, BCMA_NAND_CTLR_READY_OFFSET);
+
+	if (val & BCMA_NAND_CTLR_READY) {
+		bcma_write32(core, BCMA_NAND_CTLR_READY_OFFSET,
+			     BCMA_NAND_CTLR_READY);
+		return true;
+	}
+
+	return false;
+}
+
+static void bcma_nand_intc_set(struct brcmnand_soc *soc, bool en)
+{
+	struct bcma_nand_soc_priv *priv = soc->priv;
+	struct bcma_device *core = priv->core;
+	u32 val;
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->idm_lock, flags);
+
+	val = bcma_aread32(core, BCMA_IOCTL);
+
+	if (en)
+		val |= BCMA_NAND_INT_CTRL_READ_ENABLE;
+	else
+		val &= ~BCMA_NAND_INT_CTRL_READ_ENABLE;
+
+	bcma_awrite32(core, BCMA_IOCTL, val);
+
+	spin_unlock_irqrestore(&priv->idm_lock, flags);
+}
+
+static void bcma_nand_apb_access(struct brcmnand_soc *soc, bool prepare)
+{
+	struct bcma_nand_soc_priv *priv = soc->priv;
+	struct bcma_device *core = priv->core;
+	u32 val;
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->idm_lock, flags);
+
+	val = bcma_aread32(core, BCMA_IOCTL);
+
+	if (prepare)
+		val |= BCMA_NAND_APB_LE_MODE;
+	else
+		val &= ~BCMA_NAND_APB_LE_MODE;
+
+	bcma_awrite32(core, BCMA_IOCTL, val);
+
+	spin_unlock_irqrestore(&priv->idm_lock, flags);
+}
+
+static int bcma_nand_probe(struct bcma_device *core)
+{
+	struct device *dev = &core->dev;
+	struct bcma_nand_soc_priv *priv;
+	struct brcmnand_soc *soc;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->core = core;
+
+	soc = &priv->soc;
+	soc->nand_base = core->io_addr;
+	soc->irq = bcma_core_irq(core, 4);
+
+	soc->dev = dev;
+	soc->priv = priv;
+	soc->ctlrdy_ack = bcma_nand_intc_ack;
+	soc->ctlrdy_set_enabled = bcma_nand_intc_set;
+	soc->prepare_data_bus = bcma_nand_apb_access;
+
+	bcma_core_enable(core, 0);
+	return brcmnand_probe_dev(dev, soc);
+}
+
+static void bcma_nand_remove(struct bcma_device *core)
+{
+	brcmnand_remove_dev(&core->dev);
+}
+
+static const struct bcma_device_id bcma_nand_tbl[] = {
+	BCMA_CORE(BCMA_MANUF_BCM, BCMA_CORE_NS_NAND, BCMA_ANY_REV, BCMA_ANY_CLASS),
+	{},
+};
+MODULE_DEVICE_TABLE(bcma, bcma_nand_tbl);
+
+static struct bcma_driver bcma_nand_driver = {
+	.name		= KBUILD_MODNAME,
+	.id_table	= bcma_nand_tbl,
+	.probe		= bcma_nand_probe,
+	.remove		= bcma_nand_remove,
+};
+
+static int __init bcma_nand_init(void)
+{
+	return bcma_driver_register(&bcma_nand_driver);
+}
+
+static void __exit bcma_nand_exit(void)
+{
+	bcma_driver_unregister(&bcma_nand_driver);
+}
+
+module_init(bcma_nand_init)
+module_exit(bcma_nand_exit)
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Hauke Mehrtens");
+MODULE_DESCRIPTION("NAND driver for Broadcom bcma based SoCs");
-- 
2.1.4

Re: [PATCH 5/7] mtd: brcmnand: add bcma driver

[Brian Norris]

On Sun, May 17, 2015 at 05:41:04PM +0200, Hauke Mehrtens wrote:
> This driver registers at the bcma bus and drives the NAND core if it
> was found on this bus. The bcma bus with this NAND core is used on the
> bcm53xx and bcm47xx Northstar SoC with ARM CPU cores. The memory ranges
> are automatically detected by bcma and the irq numbers read from device
> tree by bcma bus driver.

If you're going to use device tree for part of it (IRQs) why not the
whole thing?

> This is based on the iproc driver.

This looks like you could probably get by with just using iproc_nand.c
as-is. The main NAND core is apparently MMIO-accessible on your chips,
so aren't the BCMA bits you're touching also?

> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
> ---
>  drivers/mtd/nand/Kconfig              |   8 ++
>  drivers/mtd/nand/brcmnand/Makefile    |   1 +
>  drivers/mtd/nand/brcmnand/bcma_nand.c | 153 ++++++++++++++++++++++++++++++++++
>  3 files changed, 162 insertions(+)
>  create mode 100644 drivers/mtd/nand/brcmnand/bcma_nand.c
> 
> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> index 376b538..b698212 100644
> --- a/drivers/mtd/nand/Kconfig
> +++ b/drivers/mtd/nand/Kconfig
> @@ -402,6 +402,14 @@ config MTD_NAND_BRCMNAND
>  	  originally designed for Set-Top Box but is used on various BCM7xxx,
>  	  BCM3xxx, BCM63xxx, iProc/Cygnus and more.
>  
> +config MTD_NAND_BRCMNAND_BCMA
> +	tristate "Broadcom BCMA NAND controller"
> +	depends on ARM || MIPS

depends on BCMA ?

> +	select MTD_NAND_BRCMNAND
> +	help
> +	  Enables the driver which registers Broadcom NAND controller against
> +	  bcma.

If you're going to add new Kconfigs for brcmnand, let's add a Kconfig
file under drivers/mtd/nand/brcmnand/.

Brian

> +
>  config MTD_NAND_BCM47XXNFLASH
>  	tristate "Support for NAND flash on BCM4706 BCMA bus"
>  	depends on BCMA_NFLASH
> diff --git a/drivers/mtd/nand/brcmnand/Makefile b/drivers/mtd/nand/brcmnand/Makefile
> index 3b1fbfd..8483984 100644
> --- a/drivers/mtd/nand/brcmnand/Makefile
> +++ b/drivers/mtd/nand/brcmnand/Makefile
> @@ -1,6 +1,7 @@
>  # link order matters; don't link the more generic brcmstb_nand.o before the
>  # more specific iproc_nand.o, for instance
>  obj-$(CONFIG_MTD_NAND_BRCMNAND)		+= iproc_nand.o
> +obj-$(CONFIG_MTD_NAND_BRCMNAND_BCMA)	+= bcma_nand.o
>  obj-$(CONFIG_MTD_NAND_BRCMNAND)		+= bcm63138_nand.o
>  obj-$(CONFIG_MTD_NAND_BRCMNAND)		+= brcmstb_nand.o
>  obj-$(CONFIG_MTD_NAND_BRCMNAND)		+= brcmnand.o
> diff --git a/drivers/mtd/nand/brcmnand/bcma_nand.c b/drivers/mtd/nand/brcmnand/bcma_nand.c
> new file mode 100644
> index 0000000..58380bd
> --- /dev/null
> +++ b/drivers/mtd/nand/brcmnand/bcma_nand.c
> @@ -0,0 +1,153 @@
> +/*
> + * Copyright © 2015 Broadcom Corporation
> + * Copyright 2015 Hauke Mehrtens <hauke@hauke-m.de>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/io.h>
> +#include <linux/ioport.h>
> +#include <linux/module.h>
> +#include <linux/bcma/bcma.h>
> +#include <linux/slab.h>
> +
> +#include "brcmnand.h"
> +
> +struct bcma_nand_soc_priv {
> +	struct brcmnand_soc soc;
> +	struct bcma_device *core;
> +	spinlock_t idm_lock;
> +};
> +
> +#define BCMA_NAND_CTLR_READY_OFFSET       0x0f10
> +#define BCMA_NAND_CTLR_READY              BIT(0)
> +
> +#define BCMA_NAND_APB_LE_MODE             BIT(24)
> +#define BCMA_NAND_INT_CTRL_READ_ENABLE    BIT(6)
> +
> +static bool bcma_nand_intc_ack(struct brcmnand_soc *soc)
> +{
> +	struct bcma_nand_soc_priv *priv = soc->priv;
> +	struct bcma_device *core = priv->core;
> +
> +	u32 val = bcma_read32(core, BCMA_NAND_CTLR_READY_OFFSET);
> +
> +	if (val & BCMA_NAND_CTLR_READY) {
> +		bcma_write32(core, BCMA_NAND_CTLR_READY_OFFSET,
> +			     BCMA_NAND_CTLR_READY);
> +		return true;
> +	}
> +
> +	return false;
> +}
> +
> +static void bcma_nand_intc_set(struct brcmnand_soc *soc, bool en)
> +{
> +	struct bcma_nand_soc_priv *priv = soc->priv;
> +	struct bcma_device *core = priv->core;
> +	u32 val;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&priv->idm_lock, flags);
> +
> +	val = bcma_aread32(core, BCMA_IOCTL);
> +
> +	if (en)
> +		val |= BCMA_NAND_INT_CTRL_READ_ENABLE;
> +	else
> +		val &= ~BCMA_NAND_INT_CTRL_READ_ENABLE;
> +
> +	bcma_awrite32(core, BCMA_IOCTL, val);
> +
> +	spin_unlock_irqrestore(&priv->idm_lock, flags);
> +}
> +
> +static void bcma_nand_apb_access(struct brcmnand_soc *soc, bool prepare)
> +{
> +	struct bcma_nand_soc_priv *priv = soc->priv;
> +	struct bcma_device *core = priv->core;
> +	u32 val;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&priv->idm_lock, flags);
> +
> +	val = bcma_aread32(core, BCMA_IOCTL);
> +
> +	if (prepare)
> +		val |= BCMA_NAND_APB_LE_MODE;
> +	else
> +		val &= ~BCMA_NAND_APB_LE_MODE;
> +
> +	bcma_awrite32(core, BCMA_IOCTL, val);
> +
> +	spin_unlock_irqrestore(&priv->idm_lock, flags);
> +}
> +
> +static int bcma_nand_probe(struct bcma_device *core)
> +{
> +	struct device *dev = &core->dev;
> +	struct bcma_nand_soc_priv *priv;
> +	struct brcmnand_soc *soc;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->core = core;
> +
> +	soc = &priv->soc;
> +	soc->nand_base = core->io_addr;
> +	soc->irq = bcma_core_irq(core, 4);
> +
> +	soc->dev = dev;
> +	soc->priv = priv;
> +	soc->ctlrdy_ack = bcma_nand_intc_ack;
> +	soc->ctlrdy_set_enabled = bcma_nand_intc_set;
> +	soc->prepare_data_bus = bcma_nand_apb_access;
> +
> +	bcma_core_enable(core, 0);
> +	return brcmnand_probe_dev(dev, soc);
> +}
> +
> +static void bcma_nand_remove(struct bcma_device *core)
> +{
> +	brcmnand_remove_dev(&core->dev);
> +}
> +
> +static const struct bcma_device_id bcma_nand_tbl[] = {
> +	BCMA_CORE(BCMA_MANUF_BCM, BCMA_CORE_NS_NAND, BCMA_ANY_REV, BCMA_ANY_CLASS),
> +	{},
> +};
> +MODULE_DEVICE_TABLE(bcma, bcma_nand_tbl);
> +
> +static struct bcma_driver bcma_nand_driver = {
> +	.name		= KBUILD_MODNAME,
> +	.id_table	= bcma_nand_tbl,
> +	.probe		= bcma_nand_probe,
> +	.remove		= bcma_nand_remove,
> +};
> +
> +static int __init bcma_nand_init(void)
> +{
> +	return bcma_driver_register(&bcma_nand_driver);
> +}
> +
> +static void __exit bcma_nand_exit(void)
> +{
> +	bcma_driver_unregister(&bcma_nand_driver);
> +}
> +
> +module_init(bcma_nand_init)
> +module_exit(bcma_nand_exit)
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Hauke Mehrtens");
> +MODULE_DESCRIPTION("NAND driver for Broadcom bcma based SoCs");
> -- 
> 2.1.4
> 

Re: [PATCH 5/7] mtd: brcmnand: add bcma driver

[Rafał Miłecki]

On 20 May 2015 at 02:34, Brian Norris <computersforpeace@gmail.com> wrote:
> On Sun, May 17, 2015 at 05:41:04PM +0200, Hauke Mehrtens wrote:
>> This driver registers at the bcma bus and drives the NAND core if it
>> was found on this bus. The bcma bus with this NAND core is used on the
>> bcm53xx and bcm47xx Northstar SoC with ARM CPU cores. The memory ranges
>> are automatically detected by bcma and the irq numbers read from device
>> tree by bcma bus driver.
>
> If you're going to use device tree for part of it (IRQs) why not the
> whole thing?
>
>> This is based on the iproc driver.
>
> This looks like you could probably get by with just using iproc_nand.c
> as-is. The main NAND core is apparently MMIO-accessible on your chips,
> so aren't the BCMA bits you're touching also?

That's right, in case of SoCs cores are MMIO-accessible, however I see
few reasons for writing this driver as bcma based:
1) MMIO access isn't available for bcma bus /hosted/ on PCIe devices.
By using bcma layer we write generic drivers.
2) bcma detects cores and their MMIO addresses automatically, if we
are a bit lazy, it's easier to use it rather than keep hardcoding all
addresses
3) There are some dependencies in cores initialization, e.g.
ChipCommon core usually has to be initialized first
4) bcma provides some helpers like bcma_core_enable so we don't have
to duplicate it in driver code

That said, I'm for using bcma layer, even if there is some small DT
involvement already.

Re: [PATCH 5/7] mtd: brcmnand: add bcma driver

[Brian Norris]

On Wed, May 20, 2015 at 08:39:06AM +0200, Rafał Miłecki wrote:
> On 20 May 2015 at 02:34, Brian Norris <computersforpeace@gmail.com> wrote:
> > On Sun, May 17, 2015 at 05:41:04PM +0200, Hauke Mehrtens wrote:
> >> This driver registers at the bcma bus and drives the NAND core if it
> >> was found on this bus. The bcma bus with this NAND core is used on the
> >> bcm53xx and bcm47xx Northstar SoC with ARM CPU cores. The memory ranges
> >> are automatically detected by bcma and the irq numbers read from device
> >> tree by bcma bus driver.
> >
> > If you're going to use device tree for part of it (IRQs) why not the
> > whole thing?
> >
> >> This is based on the iproc driver.
> >
> > This looks like you could probably get by with just using iproc_nand.c
> > as-is. The main NAND core is apparently MMIO-accessible on your chips,
> > so aren't the BCMA bits you're touching also?
> 
> That's right, in case of SoCs cores are MMIO-accessible, however I see
> few reasons for writing this driver as bcma based:
> 1) MMIO access isn't available for bcma bus /hosted/ on PCIe devices.
> By using bcma layer we write generic drivers.

I strongly doubt that this NAND core is ever put on a PCIe endpoint.

> 2) bcma detects cores and their MMIO addresses automatically, if we
> are a bit lazy, it's easier to use it rather than keep hardcoding all
> addresses

Laziness is a pretty bad excuse. You already have to do 60% of the work
by putting the IRQs and base NAND register range into the device tree.
Finding those remaining two register addresses is not so hard.

> 3) There are some dependencies in cores initialization, e.g.
> ChipCommon core usually has to be initialized first

Are you aware of any important dependencies? Isn't it safe to assume
that the ChipCommon core would have to be initialized way before any
peripherals?

> 4) bcma provides some helpers like bcma_core_enable so we don't have
> to duplicate it in driver code

I don't see why we need to reset/re-enable the NAND core in the kernel
at all, but if we do, this is touching the exact same registers as
iproc_nand.c is already. So it makes sense to *share* that code, and do
the same thing on both Cygnus and Northstar, etc. (And no, Cygnus can't
convert to BCMA, so we can't do 100% sharing either way.)

> That said, I'm for using bcma layer, even if there is some small DT
> involvement already.

For any reasons besides the above? Cause I'm still not convinced we need
a BCMA driver at all.

Brian

Re: [PATCH 5/7] mtd: brcmnand: add bcma driver

[Hauke Mehrtens]

On 05/20/2015 08:40 PM, Brian Norris wrote:
> On Wed, May 20, 2015 at 08:39:06AM +0200, Rafał Miłecki wrote:
>> On 20 May 2015 at 02:34, Brian Norris <computersforpeace@gmail.com> wrote:
>>> On Sun, May 17, 2015 at 05:41:04PM +0200, Hauke Mehrtens wrote:
>>>> This driver registers at the bcma bus and drives the NAND core if it
>>>> was found on this bus. The bcma bus with this NAND core is used on the
>>>> bcm53xx and bcm47xx Northstar SoC with ARM CPU cores. The memory ranges
>>>> are automatically detected by bcma and the irq numbers read from device
>>>> tree by bcma bus driver.
>>>
>>> If you're going to use device tree for part of it (IRQs) why not the
>>> whole thing?
>>>
>>>> This is based on the iproc driver.
>>>
>>> This looks like you could probably get by with just using iproc_nand.c
>>> as-is. The main NAND core is apparently MMIO-accessible on your chips,
>>> so aren't the BCMA bits you're touching also?

I will try this, I do not know if I have to reset or active the core
before using it, at least the vendor driver does so and I added it also.

>> That's right, in case of SoCs cores are MMIO-accessible, however I see
>> few reasons for writing this driver as bcma based:
>> 1) MMIO access isn't available for bcma bus /hosted/ on PCIe devices.
>> By using bcma layer we write generic drivers.
> 
> I strongly doubt that this NAND core is ever put on a PCIe endpoint.

Me too and then my driver would not work, because I am forwarding the
memory directly to the driver and nothing would change the active core.

>> 2) bcma detects cores and their MMIO addresses automatically, if we
>> are a bit lazy, it's easier to use it rather than keep hardcoding all
>> addresses
> 
> Laziness is a pretty bad excuse. You already have to do 60% of the work
> by putting the IRQs and base NAND register range into the device tree.
> Finding those remaining two register addresses is not so hard.
> 
>> 3) There are some dependencies in cores initialization, e.g.
>> ChipCommon core usually has to be initialized first
> 
> Are you aware of any important dependencies? Isn't it safe to assume
> that the ChipCommon core would have to be initialized way before any
> peripherals?

I think ChipCommon is less important on ARM, I do not came up with an
dependencies.

> 
>> 4) bcma provides some helpers like bcma_core_enable so we don't have
>> to duplicate it in driver code
> 
> I don't see why we need to reset/re-enable the NAND core in the kernel
> at all, but if we do, this is touching the exact same registers as
> iproc_nand.c is already. So it makes sense to *share* that code, and do
> the same thing on both Cygnus and Northstar, etc. (And no, Cygnus can't
> convert to BCMA, so we can't do 100% sharing either way.)

Will this Broadcom plugin to the AXI bus which bcma uses be removed from
all newer SoCs or just from some SoC lines? If it will not be there in
any more recent SoC then we should also go for the older arm SoCs to a
more device tree only approach. Is Cygnus related to Northstar Plus or
Northstar 2?

>> That said, I'm for using bcma layer, even if there is some small DT
>> involvement already.
> 
> For any reasons besides the above? Cause I'm still not convinced we need
> a BCMA driver at all.
> 
> Brian
> 

Re: [PATCH 5/7] mtd: brcmnand: add bcma driver

[Ray Jui]

Hi Hauke,

On 5/20/2015 3:10 PM, Hauke Mehrtens wrote:
> On 05/20/2015 08:40 PM, Brian Norris wrote:
>> On Wed, May 20, 2015 at 08:39:06AM +0200, Rafał Miłecki wrote:
>>> On 20 May 2015 at 02:34, Brian Norris <computersforpeace@gmail.com> wrote:
>>>> On Sun, May 17, 2015 at 05:41:04PM +0200, Hauke Mehrtens wrote:
>>>>> This driver registers at the bcma bus and drives the NAND core if it
>>>>> was found on this bus. The bcma bus with this NAND core is used on the
>>>>> bcm53xx and bcm47xx Northstar SoC with ARM CPU cores. The memory ranges
>>>>> are automatically detected by bcma and the irq numbers read from device
>>>>> tree by bcma bus driver.
>>>>
>>>> If you're going to use device tree for part of it (IRQs) why not the
>>>> whole thing?
>>>>
>>>>> This is based on the iproc driver.
>>>>
>>>> This looks like you could probably get by with just using iproc_nand.c
>>>> as-is. The main NAND core is apparently MMIO-accessible on your chips,
>>>> so aren't the BCMA bits you're touching also?
> 
> I will try this, I do not know if I have to reset or active the core
> before using it, at least the vendor driver does so and I added it also.
> 
>>> That's right, in case of SoCs cores are MMIO-accessible, however I see
>>> few reasons for writing this driver as bcma based:
>>> 1) MMIO access isn't available for bcma bus /hosted/ on PCIe devices.
>>> By using bcma layer we write generic drivers.
>>
>> I strongly doubt that this NAND core is ever put on a PCIe endpoint.
> 
> Me too and then my driver would not work, because I am forwarding the
> memory directly to the driver and nothing would change the active core.
> 
>>> 2) bcma detects cores and their MMIO addresses automatically, if we
>>> are a bit lazy, it's easier to use it rather than keep hardcoding all
>>> addresses
>>
>> Laziness is a pretty bad excuse. You already have to do 60% of the work
>> by putting the IRQs and base NAND register range into the device tree.
>> Finding those remaining two register addresses is not so hard.
>>
>>> 3) There are some dependencies in cores initialization, e.g.
>>> ChipCommon core usually has to be initialized first
>>
>> Are you aware of any important dependencies? Isn't it safe to assume
>> that the ChipCommon core would have to be initialized way before any
>> peripherals?
> 
> I think ChipCommon is less important on ARM, I do not came up with an
> dependencies.
> 
>>
>>> 4) bcma provides some helpers like bcma_core_enable so we don't have
>>> to duplicate it in driver code
>>
>> I don't see why we need to reset/re-enable the NAND core in the kernel
>> at all, but if we do, this is touching the exact same registers as
>> iproc_nand.c is already. So it makes sense to *share* that code, and do
>> the same thing on both Cygnus and Northstar, etc. (And no, Cygnus can't
>> convert to BCMA, so we can't do 100% sharing either way.)
> 
> Will this Broadcom plugin to the AXI bus which bcma uses be removed from
> all newer SoCs or just from some SoC lines? If it will not be there in
> any more recent SoC then we should also go for the older arm SoCs to a
> more device tree only approach. Is Cygnus related to Northstar Plus or
> Northstar 2?

I don't think I can give you a certain answer on this. But to my best
knowledge, I don't think we have BCMA for several of our next gen ARMv8
based SoCs.

A set of peripherals (e.g., NAND, PCIe RC, I2C, etc) are shared between
Cygnus, NS+, and NS2.

Thanks,

Ray

Re: [PATCH 5/7] mtd: brcmnand: add bcma driver

[Rafał Miłecki]

On 20 May 2015 at 20:40, Brian Norris <computersforpeace@gmail.com> wrote:
> On Wed, May 20, 2015 at 08:39:06AM +0200, Rafał Miłecki wrote:
>> On 20 May 2015 at 02:34, Brian Norris <computersforpeace@gmail.com> wrote:
>> > On Sun, May 17, 2015 at 05:41:04PM +0200, Hauke Mehrtens wrote:
>> >> This driver registers at the bcma bus and drives the NAND core if it
>> >> was found on this bus. The bcma bus with this NAND core is used on the
>> >> bcm53xx and bcm47xx Northstar SoC with ARM CPU cores. The memory ranges
>> >> are automatically detected by bcma and the irq numbers read from device
>> >> tree by bcma bus driver.
>> >
>> > If you're going to use device tree for part of it (IRQs) why not the
>> > whole thing?
>> >
>> >> This is based on the iproc driver.
>> >
>> > This looks like you could probably get by with just using iproc_nand.c
>> > as-is. The main NAND core is apparently MMIO-accessible on your chips,
>> > so aren't the BCMA bits you're touching also?
>>
>> That's right, in case of SoCs cores are MMIO-accessible, however I see
>> few reasons for writing this driver as bcma based:
>> 1) MMIO access isn't available for bcma bus /hosted/ on PCIe devices.
>> By using bcma layer we write generic drivers.
>
> I strongly doubt that this NAND core is ever put on a PCIe endpoint.

Sure, but I still don't understand why you want to bypass some layer
that helps with stuff. When I tried to implement whole SPI stuff in
bcm53xxspiflash, I was told to work on separated SPI host driver
instead. I did. When someone recently tried to workaround some SPI
host data limit, he was told to fix SPI host driver.
I don't see a reason why we're trying to skip the bcma layer for brcmnand.


>> 2) bcma detects cores and their MMIO addresses automatically, if we
>> are a bit lazy, it's easier to use it rather than keep hardcoding all
>> addresses
>
> Laziness is a pretty bad excuse. You already have to do 60% of the work
> by putting the IRQs and base NAND register range into the device tree.
> Finding those remaining two register addresses is not so hard.

Lazy I was not checking dictionary for a better word.
IRQs are indeed read from DT, because hardware doesn't provide info about them.

However it's a different story for register ranges. We read them from
device's (bus's) EEPROM, not DT. Why is it important? One Broadcom SoC
chipset may have various cores (SoC devices)
available/enabled/disabled depending on the manufacturer. It means we
*can't* store list of cores (SoC devices) per chip as it varies. An
example: some BCM4708 SoCs have 2 PCIe host cores, some have 3 of
them. Reading info about cores from hardware (EEPROM) gives you an
answer. I can't really imagine storing such info per every possible
market device nor building such DTs. I would need to ask device owner
to use bcma to read info about cores and them copy it into DT. I don't
really see the point. Please note that we are talking about every
single device model because there isn't anything like a set of few
generic PCB designs. Every manufacturer applies its own modifications.
I also can't see what's so wrong with this design. I understand you
put info about CPU in DT, but not all stuff goes there, right? E.g.
we don't put info about NAND flash connected to the controller. We
simply use ONFI (or some other semi-standards) to read chip info.
Should we now get rid of nand_flash_detect_onfi and
nand_flash_detect_jedec and store flash params in DT of every
supported device model?


>> 3) There are some dependencies in cores initialization, e.g.
>> ChipCommon core usually has to be initialized first
>
> Are you aware of any important dependencies? Isn't it safe to assume
> that the ChipCommon core would have to be initialized way before any
> peripherals?

On ARM we are left with ChipCommon only I believe. On MIPS we need to
also initialize MIPS core and on PCIe devices we have to initialize
PCIe core.


>> 4) bcma provides some helpers like bcma_core_enable so we don't have
>> to duplicate it in driver code
>
> I don't see why we need to reset/re-enable the NAND core in the kernel
> at all, but if we do, this is touching the exact same registers as
> iproc_nand.c is already. So it makes sense to *share* that code, and do
> the same thing on both Cygnus and Northstar, etc. (And no, Cygnus can't
> convert to BCMA, so we can't do 100% sharing either way.)

I believe Cygnus differs from Northstar and handles
initialization/IRQs differently. I guess it doesn't need the same core
reset routing as bcma/Northstar performs? Don't worry, converting it
to bcma is the last thing I plan to do.


>> That said, I'm for using bcma layer, even if there is some small DT
>> involvement already.
>
> For any reasons besides the above? Cause I'm still not convinced we need
> a BCMA driver at all.

Could you tell me why we *don't* want to have bcma so much?

-- 
Rafał

Re: [PATCH 5/7] mtd: brcmnand: add bcma driver

[Brian Norris]

On Thu, May 21, 2015 at 09:51:11AM +0200, Rafał Miłecki wrote:
> On 20 May 2015 at 20:40, Brian Norris <computersforpeace@gmail.com> wrote:
> > On Wed, May 20, 2015 at 08:39:06AM +0200, Rafał Miłecki wrote:
> >> On 20 May 2015 at 02:34, Brian Norris <computersforpeace@gmail.com> wrote:
> >> > On Sun, May 17, 2015 at 05:41:04PM +0200, Hauke Mehrtens wrote:
> >> >> This driver registers at the bcma bus and drives the NAND core if it
> >> >> was found on this bus. The bcma bus with this NAND core is used on the
> >> >> bcm53xx and bcm47xx Northstar SoC with ARM CPU cores. The memory ranges
> >> >> are automatically detected by bcma and the irq numbers read from device
> >> >> tree by bcma bus driver.
> >> >
> >> > If you're going to use device tree for part of it (IRQs) why not the
> >> > whole thing?
> >> >
> >> >> This is based on the iproc driver.
> >> >
> >> > This looks like you could probably get by with just using iproc_nand.c
> >> > as-is. The main NAND core is apparently MMIO-accessible on your chips,
> >> > so aren't the BCMA bits you're touching also?
> >>
> >> That's right, in case of SoCs cores are MMIO-accessible, however I see
> >> few reasons for writing this driver as bcma based:
> >> 1) MMIO access isn't available for bcma bus /hosted/ on PCIe devices.
> >> By using bcma layer we write generic drivers.
> >
> > I strongly doubt that this NAND core is ever put on a PCIe endpoint.
> 
> Sure, but I still don't understand why you want to bypass some layer
> that helps with stuff.
[snip irrelevant examples]

My motivation: don't duplicate code unnecessarily. iproc_nand is already
necessary, and it supports everything you need (see Hauke's patch, which
apparently supports these chips with no additional code).

So when you say "helps with stuff", I ask "does it really help?", and
"is it worth the duplicated driver?"

> >> 2) bcma detects cores and their MMIO addresses automatically, if we
> >> are a bit lazy, it's easier to use it rather than keep hardcoding all
> >> addresses
> >
> > Laziness is a pretty bad excuse. You already have to do 60% of the work
> > by putting the IRQs and base NAND register range into the device tree.
> > Finding those remaining two register addresses is not so hard.
> 
> Lazy I was not checking dictionary for a better word.
> IRQs are indeed read from DT, because hardware doesn't provide info about them.

Nor does it provide information about chip selects, ECC requirements or
partitioning.

> However it's a different story for register ranges. We read them from
> device's (bus's) EEPROM, not DT. Why is it important? One Broadcom SoC
> chipset may have various cores (SoC devices)
> available/enabled/disabled depending on the manufacturer. It means we
> *can't* store list of cores (SoC devices) per chip as it varies. An
> example: some BCM4708 SoCs have 2 PCIe host cores, some have 3 of
> them.

Are those all true BCM4708? Or are they bondout variations of it, with a
different name (e.g., BCM47081)?

> Reading info about cores from hardware (EEPROM) gives you an
> answer.

For BCMA to help in consolidating the other non-BCMA information into
one place, you need *everything* about the NAND setup to be the same,
except for the presence / absence of the NAND controller. That means all
manufacturers must be using BCH-8 / 512-byte step ECC, and they must
have placed it on the same chip-select.

And are you ever seeing the NAND core completely disabled? In my
experience, it has never been OTP'd out of any chip bondout.

If my previous sentence holds true, then BCMA does indeed provide you
absolutely nothing that you describe above for this case. I don't doubt
it helps you for some other cores, but I don't see that for NAND.

> I can't really imagine storing such info per every possible
> market device nor building such DTs. I would need to ask device owner
> to use bcma to read info about cores and them copy it into DT. I don't
> really see the point. Please note that we are talking about every
> single device model because there isn't anything like a set of few
> generic PCB designs. Every manufacturer applies its own modifications.

Are you sure it's done on a manufacturer-by-manufacturer basis?
Typically, I see that Broadcom chips will support a superset of features
on a base SoC, then subsequent boundouts of that SoC will provide a
subset of those features (e.g., 2 PCIe hosts instead of 3). Each of
those bondouts will have a different chip name and product ID, so
technically you would see the same set of hardware for -- hypothetically
-- all BCMabc, and a child chip of that family -- call it BCMabcd --
might disable a few cores. So depending on the number of SoC bondouts,
this may or may not be cumbersome. And particularly for NAND, I expect
the additional work may be zero.

> I also can't see what's so wrong with this design. I understand you
> put info about CPU in DT, but not all stuff goes there, right?

A *lot* of stuff goes into DT. Enough that yes, you essentially need a
new DT for every new board. So accounting for a few chip OTP options is
not really out of scope of supporting a new board/chip.

> E.g.
> we don't put info about NAND flash connected to the controller. We
> simply use ONFI (or some other semi-standards) to read chip info.
> Should we now get rid of nand_flash_detect_onfi and
> nand_flash_detect_jedec and store flash params in DT of every
> supported device model?

You're going a little bit down the straw man route of argument. That's
nothing like what I'm suggesting.

> >> That said, I'm for using bcma layer, even if there is some small DT
> >> involvement already.
> >
> > For any reasons besides the above? Cause I'm still not convinced we need
> > a BCMA driver at all.
> 
> Could you tell me why we *don't* want to have bcma so much?

It's duplicating code that already exists and supports the cores you
want. If we find that there is enough of a significant difference, then
maybe we can justify a second driver. But IIUC, Hauke is showing that it
is trivial to support your chips with trivial DT additions and zero
additional code.

--

On a slightly different track: if you really think BCMA+DT wins you a
lot over a bare DT (I'm still not convinced), then maybe there's a way
to make that work. What do you think of using BCMA to generate the DT
automatically? Either in a pre-boot shim layer before the kernel, or as
an in-kernel dynamic DT patch?

Brian

Re: [PATCH 5/7] mtd: brcmnand: add bcma driver

[Hauke Mehrtens]

On 05/27/2015 02:18 AM, Brian Norris wrote:
> On Thu, May 21, 2015 at 09:51:11AM +0200, Rafał Miłecki wrote:
>> On 20 May 2015 at 20:40, Brian Norris <computersforpeace@gmail.com> wrote:
>>> On Wed, May 20, 2015 at 08:39:06AM +0200, Rafał Miłecki wrote:
>>>> On 20 May 2015 at 02:34, Brian Norris <computersforpeace@gmail.com> wrote:
>>>>> On Sun, May 17, 2015 at 05:41:04PM +0200, Hauke Mehrtens wrote:
>>>>>> This driver registers at the bcma bus and drives the NAND core if it
>>>>>> was found on this bus. The bcma bus with this NAND core is used on the
>>>>>> bcm53xx and bcm47xx Northstar SoC with ARM CPU cores. The memory ranges
>>>>>> are automatically detected by bcma and the irq numbers read from device
>>>>>> tree by bcma bus driver.
>>>>>
>>>>> If you're going to use device tree for part of it (IRQs) why not the
>>>>> whole thing?
>>>>>
>>>>>> This is based on the iproc driver.
>>>>>
>>>>> This looks like you could probably get by with just using iproc_nand.c
>>>>> as-is. The main NAND core is apparently MMIO-accessible on your chips,
>>>>> so aren't the BCMA bits you're touching also?
>>>>
>>>> That's right, in case of SoCs cores are MMIO-accessible, however I see
>>>> few reasons for writing this driver as bcma based:
>>>> 1) MMIO access isn't available for bcma bus /hosted/ on PCIe devices.
>>>> By using bcma layer we write generic drivers.
>>>
>>> I strongly doubt that this NAND core is ever put on a PCIe endpoint.
>>
>> Sure, but I still don't understand why you want to bypass some layer
>> that helps with stuff.
> [snip irrelevant examples]
> 
> My motivation: don't duplicate code unnecessarily. iproc_nand is already
> necessary, and it supports everything you need (see Hauke's patch, which
> apparently supports these chips with no additional code).
> 
> So when you say "helps with stuff", I ask "does it really help?", and
> "is it worth the duplicated driver?"
> 
>>>> 2) bcma detects cores and their MMIO addresses automatically, if we
>>>> are a bit lazy, it's easier to use it rather than keep hardcoding all
>>>> addresses
>>>
>>> Laziness is a pretty bad excuse. You already have to do 60% of the work
>>> by putting the IRQs and base NAND register range into the device tree.
>>> Finding those remaining two register addresses is not so hard.
>>
>> Lazy I was not checking dictionary for a better word.
>> IRQs are indeed read from DT, because hardware doesn't provide info about them.
> 
> Nor does it provide information about chip selects, ECC requirements or
> partitioning.
> 
>> However it's a different story for register ranges. We read them from
>> device's (bus's) EEPROM, not DT. Why is it important? One Broadcom SoC
>> chipset may have various cores (SoC devices)
>> available/enabled/disabled depending on the manufacturer. It means we
>> *can't* store list of cores (SoC devices) per chip as it varies. An
>> example: some BCM4708 SoCs have 2 PCIe host cores, some have 3 of
>> them.
> 
> Are those all true BCM4708? Or are they bondout variations of it, with a
> different name (e.g., BCM47081)?

They have a different marketing name and a different package number on
the technical side.
> 
>> Reading info about cores from hardware (EEPROM) gives you an
>> answer.
> 
> For BCMA to help in consolidating the other non-BCMA information into
> one place, you need *everything* about the NAND setup to be the same,
> except for the presence / absence of the NAND controller. That means all
> manufacturers must be using BCH-8 / 512-byte step ECC, and they must
> have placed it on the same chip-select.
> 
> And are you ever seeing the NAND core completely disabled? In my
> experience, it has never been OTP'd out of any chip bondout.

I haven't seen it disabled on the ARM SoCs

> If my previous sentence holds true, then BCMA does indeed provide you
> absolutely nothing that you describe above for this case. I don't doubt
> it helps you for some other cores, but I don't see that for NAND.
> 
>> I can't really imagine storing such info per every possible
>> market device nor building such DTs. I would need to ask device owner
>> to use bcma to read info about cores and them copy it into DT. I don't
>> really see the point. Please note that we are talking about every
>> single device model because there isn't anything like a set of few
>> generic PCB designs. Every manufacturer applies its own modifications.
> 
> Are you sure it's done on a manufacturer-by-manufacturer basis?
> Typically, I see that Broadcom chips will support a superset of features
> on a base SoC, then subsequent boundouts of that SoC will provide a
> subset of those features (e.g., 2 PCIe hosts instead of 3). Each of
> those bondouts will have a different chip name and product ID, so
> technically you would see the same set of hardware for -- hypothetically
> -- all BCMabc, and a child chip of that family -- call it BCMabcd --
> might disable a few cores. So depending on the number of SoC bondouts,
> this may or may not be cumbersome. And particularly for NAND, I expect
> the additional work may be zero.
> 
>> I also can't see what's so wrong with this design. I understand you
>> put info about CPU in DT, but not all stuff goes there, right?
> 
> A *lot* of stuff goes into DT. Enough that yes, you essentially need a
> new DT for every new board. So accounting for a few chip OTP options is
> not really out of scope of supporting a new board/chip.
> 
>> E.g.
>> we don't put info about NAND flash connected to the controller. We
>> simply use ONFI (or some other semi-standards) to read chip info.
>> Should we now get rid of nand_flash_detect_onfi and
>> nand_flash_detect_jedec and store flash params in DT of every
>> supported device model?
> 
> You're going a little bit down the straw man route of argument. That's
> nothing like what I'm suggesting.
> 
>>>> That said, I'm for using bcma layer, even if there is some small DT
>>>> involvement already.
>>>
>>> For any reasons besides the above? Cause I'm still not convinced we need
>>> a BCMA driver at all.
>>
>> Could you tell me why we *don't* want to have bcma so much?
> 
> It's duplicating code that already exists and supports the cores you
> want. If we find that there is enough of a significant difference, then
> maybe we can justify a second driver. But IIUC, Hauke is showing that it
> is trivial to support your chips with trivial DT additions and zero
> additional code.
> 
> --
> 
> On a slightly different track: if you really think BCMA+DT wins you a
> lot over a bare DT (I'm still not convinced), then maybe there's a way
> to make that work. What do you think of using BCMA to generate the DT
> automatically? Either in a pre-boot shim layer before the kernel, or as
> an in-kernel dynamic DT patch?

I do not think that would work, because bcma still has to support the
Broadcom PCIe wifi card with soft MAC wifi and the MIPS SoCs which are
not using device tree and havning both interfaces does not make sense to me.

Hauke

[PATCH 6/7] mtd: brcmnand: run bcm47xxpart part parser in addition

[Hauke Mehrtens]

On the bcm53xx and bcm47xx SoC the bcm47xxpart partition parser is able
to parse the partitions automatically by reading some special header
used on these SoCs flash partition and some guessing. Without this
patch the default list is used, with this patch this partition parser
is used if the others haven't found a partition.

Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 drivers/mtd/nand/brcmnand/brcmnand.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/brcmnand/brcmnand.c b/drivers/mtd/nand/brcmnand/brcmnand.c
index 468a212..cc5dc1e 100644
--- a/drivers/mtd/nand/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/brcmnand/brcmnand.c
@@ -1879,6 +1879,9 @@ static int brcmnand_setup_dev(struct brcmnand_host *host)
 	return 0;
 }
 
+static const char * const probes[] = {"cmdlinepart", "ofpart", "bcm47xxpart",
+				      NULL};
+
 static int brcmnand_init_cs(struct brcmnand_host *host)
 {
 	struct brcmnand_controller *ctrl = host->ctrl;
@@ -1956,7 +1959,7 @@ static int brcmnand_init_cs(struct brcmnand_host *host)
 	if (nand_scan_tail(mtd))
 		return -ENXIO;
 
-	return mtd_device_parse_register(mtd, NULL, &ppdata, NULL, 0);
+	return mtd_device_parse_register(mtd, probes, &ppdata, NULL, 0);
 }
 
 static void brcmnand_save_restore_cs_config(struct brcmnand_host *host,
-- 
2.1.4

Re: [PATCH 6/7] mtd: brcmnand: run bcm47xxpart part parser in addition

[Jonas Gorski]

Hi Hauke,

On Sun, May 17, 2015 at 5:41 PM, Hauke Mehrtens <hauke@hauke-m.de> wrote:
> On the bcm53xx and bcm47xx SoC the bcm47xxpart partition parser is able
> to parse the partitions automatically by reading some special header
> used on these SoCs flash partition and some guessing. Without this
> patch the default list is used, with this patch this partition parser
> is used if the others haven't found a partition.
>
> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
> ---
>  drivers/mtd/nand/brcmnand/brcmnand.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/nand/brcmnand/brcmnand.c b/drivers/mtd/nand/brcmnand/brcmnand.c
> index 468a212..cc5dc1e 100644
> --- a/drivers/mtd/nand/brcmnand/brcmnand.c
> +++ b/drivers/mtd/nand/brcmnand/brcmnand.c
> @@ -1879,6 +1879,9 @@ static int brcmnand_setup_dev(struct brcmnand_host *host)
>         return 0;
>  }
>
> +static const char * const probes[] = {"cmdlinepart", "ofpart", "bcm47xxpart",
> +                                     NULL};
> +

I know this is a bit more work, but how about moving the
"linux,part-probe" parsing code from drivers/mtd/maps/physmap_of.c to
mtd_device_parse_register?

then mtd_device_parse_register could do something like

 if (!probes && ppdata && ppdata->of_node)
    probes = of_get_probes(ppdata->of_node);

and you could just add a linux,part-probe = "..."; property to the dts files

(It looks too easy/obvious so I guess I'm overlooking something fundamental ;P)

>  static int brcmnand_init_cs(struct brcmnand_host *host)
>  {
>         struct brcmnand_controller *ctrl = host->ctrl;
> @@ -1956,7 +1959,7 @@ static int brcmnand_init_cs(struct brcmnand_host *host)
>         if (nand_scan_tail(mtd))
>                 return -ENXIO;
>
> -       return mtd_device_parse_register(mtd, NULL, &ppdata, NULL, 0);
> +       return mtd_device_parse_register(mtd, probes, &ppdata, NULL, 0);
>  }

Then you wouldn't need to change this at all.


Regards
Jonas

Re: [PATCH 6/7] mtd: brcmnand: run bcm47xxpart part parser in addition

[Hauke Mehrtens]

On 05/17/2015 06:05 PM, Jonas Gorski wrote:
> Hi Hauke,
> 
> On Sun, May 17, 2015 at 5:41 PM, Hauke Mehrtens <hauke@hauke-m.de> wrote:
>> On the bcm53xx and bcm47xx SoC the bcm47xxpart partition parser is able
>> to parse the partitions automatically by reading some special header
>> used on these SoCs flash partition and some guessing. Without this
>> patch the default list is used, with this patch this partition parser
>> is used if the others haven't found a partition.
>>
>> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
>> ---
>>  drivers/mtd/nand/brcmnand/brcmnand.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mtd/nand/brcmnand/brcmnand.c b/drivers/mtd/nand/brcmnand/brcmnand.c
>> index 468a212..cc5dc1e 100644
>> --- a/drivers/mtd/nand/brcmnand/brcmnand.c
>> +++ b/drivers/mtd/nand/brcmnand/brcmnand.c
>> @@ -1879,6 +1879,9 @@ static int brcmnand_setup_dev(struct brcmnand_host *host)
>>         return 0;
>>  }
>>
>> +static const char * const probes[] = {"cmdlinepart", "ofpart", "bcm47xxpart",
>> +                                     NULL};
>> +
> 
> I know this is a bit more work, but how about moving the
> "linux,part-probe" parsing code from drivers/mtd/maps/physmap_of.c to
> mtd_device_parse_register?
> 
> then mtd_device_parse_register could do something like
> 
>  if (!probes && ppdata && ppdata->of_node)
>     probes = of_get_probes(ppdata->of_node);
> 
> and you could just add a linux,part-probe = "..."; property to the dts files
> 
> (It looks too easy/obvious so I guess I'm overlooking something fundamental ;P)
> 

Yes I will try to get this done in a generic way through device tree. I
was already searching for a way to provide the partition parser from
device tree and was wondering why it was not there.

I will send an independent patch which adds this feature and this patch
from this series can be dropped.

Hauke

[PATCH 7/7] ARM: BCM5301X: add NAND flash chip description

[Hauke Mehrtens]

This adds the NAND flash chip description for a standard chip found
connected to this SoC. The IRQ is fetched from the device tree
definition of the axi bus for this core and the memory ranges are
automatically probed.

Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 arch/arm/boot/dts/bcm4708-luxul-xwc-1000.dts | 12 +++++-------
 arch/arm/boot/dts/bcm5301x.dtsi              | 19 +++++++++++++++++++
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/arch/arm/boot/dts/bcm4708-luxul-xwc-1000.dts b/arch/arm/boot/dts/bcm4708-luxul-xwc-1000.dts
index 946c728..0529682 100644
--- a/arch/arm/boot/dts/bcm4708-luxul-xwc-1000.dts
+++ b/arch/arm/boot/dts/bcm4708-luxul-xwc-1000.dts
@@ -25,13 +25,11 @@
 
 	axi@18000000 {
 		nand@28000 {
-			reg = <0x00028000 0x1000>;
-			#address-cells = <1>;
-			#size-cells = <1>;
-
-			partition@0 {
-				label = "ubi";
-				reg = <0x00000000 0x08000000>;
+			nandcs@0 {
+				partition@0 {
+					label = "ubi";
+					reg = <0x00000000 0x08000000>;
+				};
 			};
 		};
 	};
diff --git a/arch/arm/boot/dts/bcm5301x.dtsi b/arch/arm/boot/dts/bcm5301x.dtsi
index 78aec62..1bccb59 100644
--- a/arch/arm/boot/dts/bcm5301x.dtsi
+++ b/arch/arm/boot/dts/bcm5301x.dtsi
@@ -142,5 +142,24 @@
 			gpio-controller;
 			#gpio-cells = <2>;
 		};
+
+		nand: nand@28000 {
+			reg = <0x00028000 0x1000>;
+
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			brcm,nand-has-wp;
+
+			nandcs@0 {
+				compatible = "brcm,nandcs";
+				reg = <0>;
+				#address-cells = <1>;
+				#size-cells = <1>;
+
+				nand-ecc-strength = <8>;
+				nand-ecc-step-size = <512>;
+			};
+		};
 	};
 };
-- 
2.1.4