[RFC 0/3] mtd: bcm47xxnflash: adding support for new NANDs

[RFC 0/3] mtd: bcm47xxnflash: adding support for new NANDs

[Rafał Miłecki]

New Broadcom SoCs have NAND flashes attached & programmed in a totally different
way. Instead of accessing them with help of ChipCommon core, they can be used
directly.

To support them we can extend bcm47xxnflash in the way this patches implements.
However almost nothing in the code will be shared between support for old and
new devices.

How should we proceed? Implement it that way anyway? Or maybe writing a
separated driver (bcm53xxnflash?) would be a better idea?

Rafał Miłecki (3):
  Revert "mtd: bcm47xxnflash: Use module_platform_driver"
  mtd: bcm47xxnflash: add separated config for platform driver
  mtd: bcm47xxnflash: prepare for adding BCMA driver

 drivers/mtd/nand/Kconfig                | 25 ++++++++---
 drivers/mtd/nand/bcm47xxnflash/Makefile |  2 +-
 drivers/mtd/nand/bcm47xxnflash/main.c   | 80 ++++++++++++++++++++++++++++++++-
 3 files changed, 100 insertions(+), 7 deletions(-)

-- 
1.8.4.5

[RFC 1/3] Revert "mtd: bcm47xxnflash: Use module_platform_driver"

[Rafał Miłecki]

This reverts commit 994bbd0e91c0dfa4dcda9097b0716607aeec5470.

We're going to add support for more NAND flashes and the new ones are
not registered as platform devices. So we need our own init function.
---
 drivers/mtd/nand/bcm47xxnflash/main.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/bcm47xxnflash/main.c b/drivers/mtd/nand/bcm47xxnflash/main.c
index 1074459..52611ef 100644
--- a/drivers/mtd/nand/bcm47xxnflash/main.c
+++ b/drivers/mtd/nand/bcm47xxnflash/main.c
@@ -77,4 +77,22 @@ static struct platform_driver bcm47xxnflash_driver = {
 	},
 };
 
-module_platform_driver(bcm47xxnflash_driver);
+static int __init bcm47xxnflash_init(void)
+{
+	int err;
+
+	err = platform_driver_register(&bcm47xxnflash_driver);
+	if (err)
+		pr_err("Failed to register bcm47xx nand flash driver: %d\n",
+		       err);
+
+	return err;
+}
+
+static void __exit bcm47xxnflash_exit(void)
+{
+	platform_driver_unregister(&bcm47xxnflash_driver);
+}
+
+module_init(bcm47xxnflash_init);
+module_exit(bcm47xxnflash_exit);
-- 
1.8.4.5

[RFC 2/3] mtd: bcm47xxnflash: add separated config for platform driver

[Rafał Miłecki]

Adding support for new NANDs requires registering as BCMA driver, so we
will need separated configs.
---
 drivers/mtd/nand/Kconfig                | 18 +++++++++++++-----
 drivers/mtd/nand/bcm47xxnflash/Makefile |  2 +-
 drivers/mtd/nand/bcm47xxnflash/main.c   | 12 +++++++++++-
 3 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index a5bb738..a3e8ad2 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -390,12 +390,20 @@ config MTD_NAND_GPMI_NAND
 	 the GPMI.
 
 config MTD_NAND_BCM47XXNFLASH
-	tristate "Support for NAND flash on BCM4706 BCMA bus"
-	depends on BCMA_NFLASH
+	tristate "Support for NAND flash memories on BCMA bus"
+	depends on BCMA
 	help
-	  BCMA bus can have various flash memories attached, they are
-	  registered by bcma as platform devices. This enables driver for
-	  NAND flash memories. For now only BCM4706 is supported.
+	  BCMA bus can have various flash memories attached including
+	  NAND type. Depending on SoC version they can be attached to
+	  the ChipCommon core or available as separated device.
+
+config MTD_NAND_BCM47XXNFLASH_PLATFORM
+	bool "NAND attached to the ChipCommon core"
+	depends on MTD_NAND_BCM47XXNFLASH && BCMA_NFLASH
+	help
+	  Such flashes were probably used on MIPS based SoCs only. The
+	  most common was BCM4706, but it rarely could be also found on
+	  BCM5357.
 
 config MTD_NAND_PLATFORM
 	tristate "Support for generic platform NAND driver"
diff --git a/drivers/mtd/nand/bcm47xxnflash/Makefile b/drivers/mtd/nand/bcm47xxnflash/Makefile
index f05b119..4e755f0 100644
--- a/drivers/mtd/nand/bcm47xxnflash/Makefile
+++ b/drivers/mtd/nand/bcm47xxnflash/Makefile
@@ -1,4 +1,4 @@
 bcm47xxnflash-y				+= main.o
-bcm47xxnflash-y				+= ops_bcm4706.o
+bcm47xxnflash-$(CONFIG_MTD_NAND_BCM47XXNFLASH_PLATFORM)	+= ops_bcm4706.o
 
 obj-$(CONFIG_MTD_NAND_BCM47XXNFLASH)	+= bcm47xxnflash.o
diff --git a/drivers/mtd/nand/bcm47xxnflash/main.c b/drivers/mtd/nand/bcm47xxnflash/main.c
index 52611ef..c9d9b3f 100644
--- a/drivers/mtd/nand/bcm47xxnflash/main.c
+++ b/drivers/mtd/nand/bcm47xxnflash/main.c
@@ -21,6 +21,7 @@ MODULE_DESCRIPTION("NAND flash driver for BCMA bus");
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Rafał Miłecki");
 
+#if IS_ENABLED(CONFIG_MTD_NAND_BCM47XXNFLASH_PLATFORM)
 static const char *probes[] = { "bcm47xxpart", NULL };
 
 static int bcm47xxnflash_probe(struct platform_device *pdev)
@@ -76,22 +77,31 @@ static struct platform_driver bcm47xxnflash_driver = {
 		.owner = THIS_MODULE,
 	},
 };
+#endif
+
+/**************************************************
+ * Init & exit
+ **************************************************/
 
 static int __init bcm47xxnflash_init(void)
 {
-	int err;
+	int err = 0;
 
+#if IS_ENABLED(CONFIG_MTD_NAND_BCM47XXNFLASH_PLATFORM)
 	err = platform_driver_register(&bcm47xxnflash_driver);
 	if (err)
 		pr_err("Failed to register bcm47xx nand flash driver: %d\n",
 		       err);
+#endif
 
 	return err;
 }
 
 static void __exit bcm47xxnflash_exit(void)
 {
+#if IS_ENABLED(CONFIG_MTD_NAND_BCM47XXNFLASH_PLATFORM)
 	platform_driver_unregister(&bcm47xxnflash_driver);
+#endif
 }
 
 module_init(bcm47xxnflash_init);
-- 
1.8.4.5

[RFC 3/3] mtd: bcm47xxnflash: prepare for adding BCMA driver

[Rafał Miłecki]

---
 drivers/mtd/nand/Kconfig              |  7 +++++
 drivers/mtd/nand/bcm47xxnflash/main.c | 56 +++++++++++++++++++++++++++++++++--
 2 files changed, 60 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index a3e8ad2..fecde1e 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -405,6 +405,13 @@ config MTD_NAND_BCM47XXNFLASH_PLATFORM
 	  most common was BCM4706, but it rarely could be also found on
 	  BCM5357.
 
+config MTD_NAND_BCM47XXNFLASH_BCMA
+	bool "NAND attached to BCMA as standalone core"
+	depends on MTD_NAND_BCM47XXNFLASH && BROKEN
+	help
+	  New SoCs (most probably only these ARM based) have NAND
+	  flash as separated bus core.
+
 config MTD_NAND_PLATFORM
 	tristate "Support for generic platform NAND driver"
 	depends on HAS_IOMEM
diff --git a/drivers/mtd/nand/bcm47xxnflash/main.c b/drivers/mtd/nand/bcm47xxnflash/main.c
index c9d9b3f..aebe2d5 100644
--- a/drivers/mtd/nand/bcm47xxnflash/main.c
+++ b/drivers/mtd/nand/bcm47xxnflash/main.c
@@ -80,6 +80,36 @@ static struct platform_driver bcm47xxnflash_driver = {
 #endif
 
 /**************************************************
+ * BCMA
+ **************************************************/
+
+#if IS_ENABLED(CONFIG_MTD_NAND_BCM47XXNFLASH_BCMA)
+static const struct bcma_device_id bcm47xxnflash_bcma_tbl[] = {
+	BCMA_CORE(BCMA_MANUF_BCM, BCMA_CORE_NAND, BCMA_ANY_REV, BCMA_ANY_CLASS),
+	BCMA_CORETABLE_END
+};
+MODULE_DEVICE_TABLE(bcma, bcm47xxnflash_bcma_tbl);
+
+static int bcm47xxnflash_bcma_probe(struct bcma_device *core)
+{
+	pr_info("Device ID: 0x%08X\n", bcma_read32(core, 0x194));
+
+	return -ENOTSUPP;
+}
+
+static void bcm47xxnflash_bcma_remove(struct bcma_device *core)
+{
+}
+
+static struct bcma_driver bcm47xxnflash_bcma_driver = {
+	.name		= KBUILD_MODNAME,
+	.id_table	= bcm47xxnflash_bcma_tbl,
+	.probe		= bcm47xxnflash_bcma_probe,
+	.remove		= bcm47xxnflash_bcma_remove,
+};
+#endif
+
+/**************************************************
  * Init & exit
  **************************************************/
 
@@ -89,16 +119,36 @@ static int __init bcm47xxnflash_init(void)
 
 #if IS_ENABLED(CONFIG_MTD_NAND_BCM47XXNFLASH_PLATFORM)
 	err = platform_driver_register(&bcm47xxnflash_driver);
-	if (err)
-		pr_err("Failed to register bcm47xx nand flash driver: %d\n",
-		       err);
+	if (err) {
+		pr_err("Failed to register platform driver: %d\n", err);
+		return err;
+	}
+#endif
+
+#if IS_ENABLED(CONFIG_MTD_NAND_BCM47XXNFLASH_BCMA)
+	err = bcma_driver_register(&bcm47xxnflash_bcma_driver);
+	if (err) {
+		pr_err("Failed to register bcma driver: %d\n", err);
+		goto err_plat_unregister;
+	}
 #endif
 
+	return 0;
+
+#if IS_ENABLED(CONFIG_MTD_NAND_BCM47XXNFLASH_BCMA)
+err_plat_unregister:
+#endif
+#if IS_ENABLED(CONFIG_MTD_NAND_BCM47XXNFLASH_PLATFORM)
+	platform_driver_unregister(&bcm47xxnflash_driver);
+#endif
 	return err;
 }
 
 static void __exit bcm47xxnflash_exit(void)
 {
+#if IS_ENABLED(CONFIG_MTD_NAND_BCM47XXNFLASH_BCMA)
+	bcma_driver_unregister(&bcm47xxnflash_bcma_driver);
+#endif
 #if IS_ENABLED(CONFIG_MTD_NAND_BCM47XXNFLASH_PLATFORM)
 	platform_driver_unregister(&bcm47xxnflash_driver);
 #endif
-- 
1.8.4.5

Re: [RFC 0/3] mtd: bcm47xxnflash: adding support for new NANDs

[Hauke Mehrtens]

On 02/23/2014 12:26 PM, Rafał Miłecki wrote:
> New Broadcom SoCs have NAND flashes attached & programmed in a totally different
> way. Instead of accessing them with help of ChipCommon core, they can be used
> directly.
> 
> To support them we can extend bcm47xxnflash in the way this patches implements.
> However almost nothing in the code will be shared between support for old and
> new devices.
> 
> How should we proceed? Implement it that way anyway? Or maybe writing a
> separated driver (bcm53xxnflash?) would be a better idea?
> 
Hi,

if this controller is completely different I would suggest writing a new
driver for that controller. Extending the existing driver would make if
just more complex.

Hauke

Re: [RFC 0/3] mtd: bcm47xxnflash: adding support for new NANDs

[Brian Norris]

Hi Rafal,

On Sun, Feb 23, 2014 at 12:31:00PM +0100, Hauke Mehrtens wrote:
> On 02/23/2014 12:26 PM, Rafał Miłecki wrote:
> > New Broadcom SoCs have NAND flashes attached & programmed in a totally different
> > way. Instead of accessing them with help of ChipCommon core, they can be used
> > directly.
> > 
> > To support them we can extend bcm47xxnflash in the way this patches implements.
> > However almost nothing in the code will be shared between support for old and
> > new devices.
> > 
> > How should we proceed? Implement it that way anyway? Or maybe writing a
> > separated driver (bcm53xxnflash?) would be a better idea?
> 
> if this controller is completely different I would suggest writing a new
> driver for that controller. Extending the existing driver would make if
> just more complex.

Judging by your (mostly non-functional) patch:

  [WIP][FYI] mtd: bcm53xxnflash: add driver for NAND on new BCMA SoCs

it looks like your hardware is very similar to a line of NAND
controllers that I support at Broadcom. I'm planning to upstream the
driver eventually, and it is much more mature than what you have there.
Perhaps you can wait to integrate with it?

The main difference between my driver and what I see you doing for 53xx
is that you require the bcma_* accessors, whereas my platforms just
perform direct memory-mapped register access.

Brian

[RFC 3/3] mtd: bcm47xxnflash: prepare for adding BCMA driver

[Rafał Miłecki]

---
 drivers/mtd/nand/Kconfig              |  7 +++++
 drivers/mtd/nand/bcm47xxnflash/main.c | 56 +++++++++++++++++++++++++++++++++--
 2 files changed, 60 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index a3e8ad2..fecde1e 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -405,6 +405,13 @@ config MTD_NAND_BCM47XXNFLASH_PLATFORM
 	  most common was BCM4706, but it rarely could be also found on
 	  BCM5357.
 
+config MTD_NAND_BCM47XXNFLASH_BCMA
+	bool "NAND attached to BCMA as standalone core"
+	depends on MTD_NAND_BCM47XXNFLASH && BROKEN
+	help
+	  New SoCs (most probably only these ARM based) have NAND
+	  flash as separated bus core.
+
 config MTD_NAND_PLATFORM
 	tristate "Support for generic platform NAND driver"
 	depends on HAS_IOMEM
diff --git a/drivers/mtd/nand/bcm47xxnflash/main.c b/drivers/mtd/nand/bcm47xxnflash/main.c
index c9d9b3f..aebe2d5 100644
--- a/drivers/mtd/nand/bcm47xxnflash/main.c
+++ b/drivers/mtd/nand/bcm47xxnflash/main.c
@@ -80,6 +80,36 @@ static struct platform_driver bcm47xxnflash_driver = {
 #endif
 
 /**************************************************
+ * BCMA
+ **************************************************/
+
+#if IS_ENABLED(CONFIG_MTD_NAND_BCM47XXNFLASH_BCMA)
+static const struct bcma_device_id bcm47xxnflash_bcma_tbl[] = {
+	BCMA_CORE(BCMA_MANUF_BCM, BCMA_CORE_NAND, BCMA_ANY_REV, BCMA_ANY_CLASS),
+	BCMA_CORETABLE_END
+};
+MODULE_DEVICE_TABLE(bcma, bcm47xxnflash_bcma_tbl);
+
+static int bcm47xxnflash_bcma_probe(struct bcma_device *core)
+{
+	pr_info("Device ID: 0x%08X\n", bcma_read32(core, 0x194));
+
+	return -ENOTSUPP;
+}
+
+static void bcm47xxnflash_bcma_remove(struct bcma_device *core)
+{
+}
+
+static struct bcma_driver bcm47xxnflash_bcma_driver = {
+	.name		= KBUILD_MODNAME,
+	.id_table	= bcm47xxnflash_bcma_tbl,
+	.probe		= bcm47xxnflash_bcma_probe,
+	.remove		= bcm47xxnflash_bcma_remove,
+};
+#endif
+
+/**************************************************
  * Init & exit
  **************************************************/
 
@@ -89,16 +119,36 @@ static int __init bcm47xxnflash_init(void)
 
 #if IS_ENABLED(CONFIG_MTD_NAND_BCM47XXNFLASH_PLATFORM)
 	err = platform_driver_register(&bcm47xxnflash_driver);
-	if (err)
-		pr_err("Failed to register bcm47xx nand flash driver: %d\n",
-		       err);
+	if (err) {
+		pr_err("Failed to register platform driver: %d\n", err);
+		return err;
+	}
+#endif
+
+#if IS_ENABLED(CONFIG_MTD_NAND_BCM47XXNFLASH_BCMA)
+	err = bcma_driver_register(&bcm47xxnflash_bcma_driver);
+	if (err) {
+		pr_err("Failed to register bcma driver: %d\n", err);
+		goto err_plat_unregister;
+	}
 #endif
 
+	return 0;
+
+#if IS_ENABLED(CONFIG_MTD_NAND_BCM47XXNFLASH_BCMA)
+err_plat_unregister:
+#endif
+#if IS_ENABLED(CONFIG_MTD_NAND_BCM47XXNFLASH_PLATFORM)
+	platform_driver_unregister(&bcm47xxnflash_driver);
+#endif
 	return err;
 }
 
 static void __exit bcm47xxnflash_exit(void)
 {
+#if IS_ENABLED(CONFIG_MTD_NAND_BCM47XXNFLASH_BCMA)
+	bcma_driver_unregister(&bcm47xxnflash_bcma_driver);
+#endif
 #if IS_ENABLED(CONFIG_MTD_NAND_BCM47XXNFLASH_PLATFORM)
 	platform_driver_unregister(&bcm47xxnflash_driver);
 #endif
-- 
1.8.4.5