Skip to contentSkip to conversation

mtd: make onenand/generic.c more generic

PATCH In review Magnus Damm 3 participants 7 messages Aug 4, 2009 – Aug 7, 2009

Subsystems (7)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 mtd: make onenand/generic.c more generic

From: Magnus Damm <[email protected]>

This patch removes the ARM dependency from the generic "onenand" platform device driver. This change makes the driver useful for other architectures as well. Needed for the SuperH kfr2r09 board.

Apart from the obvious Kconfig bits, the most important change is the move away from ARM specific includes and platform data. Together with this change the only in-tree board code gets an update, and the driver name is also changed gracefully break potential out of tree drivers.

The driver is also updated to allow NULL as platform data together with a few changes to make use of resource_size() and dev_name().

---

Tested on the sh7724 kfr2r09 board with a separate platform data patch.

 arch/arm/mach-omap2/board-apollon.c |    4 ++--
 drivers/mtd/onenand/Kconfig         |    1 -
 drivers/mtd/onenand/generic.c       |   24 ++++++++++++++----------
 include/linux/mtd/onenand.h         |    8 ++++++++
 4 files changed, 24 insertions(+), 13 deletions(-)
arch/arm/mach-omap2/board-apollon.c +2 -2
@@ -87,7 +87,7 @@ static struct mtd_partition apollon_part
 	},
 };
 
-static struct flash_platform_data apollon_flash_data = {
+static struct onenand_platform_data apollon_flash_data = {
 	.parts		= apollon_partitions,
 	.nr_parts	= ARRAY_SIZE(apollon_partitions),
 };
@@ -99,7 +99,7 @@ static struct resource apollon_flash_res
 };
 
 static struct platform_device apollon_onenand_device = {
-	.name		= "onenand",
+	.name		= "onenand-flash",
 	.id		= -1,
 	.dev		= {
 		.platform_data	= &apollon_flash_data,
drivers/mtd/onenand/Kconfig +0 -1
@@ -23,7 +23,6 @@ config MTD_ONENAND_VERIFY_WRITE
 
 config MTD_ONENAND_GENERIC
 	tristate "OneNAND Flash device via platform device driver"
-	depends on ARM
 	help
 	  Support for OneNAND flash via platform device driver.
 
drivers/mtd/onenand/generic.c +14 -10
@@ -19,12 +19,16 @@
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/onenand.h>
 #include <linux/mtd/partitions.h>
-
 #include <asm/io.h>
-#include <asm/mach/flash.h>
-
-#define DRIVER_NAME	"onenand"
 
+/*
+ * Note: Driver name and platform data format have been updated!
+ *
+ * This version of the driver is named "onenand-flash" and takes struct
+ * onenand_platform_data as platform data. The old ARM-specific version
+ * with the name "onenand" used to take struct flash_platform_data.
+ */
+#define DRIVER_NAME	"onenand-flash"
 
 #ifdef CONFIG_MTD_PARTITIONS
 static const char *part_probes[] = { "cmdlinepart", NULL,  };
@@ -39,16 +43,16 @@ struct onenand_info {
 static int __devinit generic_onenand_probe(struct platform_device *pdev)
 {
 	struct onenand_info *info;
-	struct flash_platform_data *pdata = pdev->dev.platform_data;
+	struct onenand_platform_data *pdata = pdev->dev.platform_data;
 	struct resource *res = pdev->resource;
-	unsigned long size = res->end - res->start + 1;
+	unsigned long size = resource_size(res);
 	int err;
 
 	info = kzalloc(sizeof(struct onenand_info), GFP_KERNEL);
 	if (!info)
 		return -ENOMEM;
 
-	if (!request_mem_region(res->start, size, pdev->dev.driver->name)) {
+	if (!request_mem_region(res->start, size, dev_name(&pdev->dev))) {
 		err = -EBUSY;
 		goto out_free_info;
 	}
@@ -59,7 +63,7 @@ static int __devinit generic_onenand_pro
 		goto out_release_mem_region;
 	}
 
-	info->onenand.mmcontrol = pdata->mmcontrol;
+	info->onenand.mmcontrol = pdata ? pdata->mmcontrol : 0;
 	info->onenand.irq = platform_get_irq(pdev, 0);
 
 	info->mtd.name = dev_name(&pdev->dev);
@@ -75,7 +79,7 @@ static int __devinit generic_onenand_pro
 	err = parse_mtd_partitions(&info->mtd, part_probes, &info->parts, 0);
 	if (err > 0)
 		add_mtd_partitions(&info->mtd, info->parts, err);
-	else if (err <= 0 && pdata->parts)
+	else if (err <= 0 && pdata && pdata->parts)
 		add_mtd_partitions(&info->mtd, pdata->parts, pdata->nr_parts);
 	else
 #endif
@@ -99,7 +103,7 @@ static int __devexit generic_onenand_rem
 {
 	struct onenand_info *info = platform_get_drvdata(pdev);
 	struct resource *res = pdev->resource;
-	unsigned long size = res->end - res->start + 1;
+	unsigned long size = resource_size(res);
 
 	platform_set_drvdata(pdev, NULL);
 
include/linux/mtd/onenand.h +8 -0
@@ -214,4 +214,12 @@ unsigned onenand_block(struct onenand_ch
 loff_t onenand_addr(struct onenand_chip *this, int block);
 int flexonenand_region(struct mtd_info *mtd, loff_t addr);
 
+struct mtd_partition;
+
+struct onenand_platform_data {
+	void		(*mmcontrol)(struct mtd_info *mtd, int sync_read);
+	struct mtd_partition *parts;
+	unsigned int	nr_parts;
+};
+
 #endif	/* __LINUX_MTD_ONENAND_H */
#1 lore raw

Hi

Good idea add the onenand_platform_data, but dont' agree the renaming the onenand-flash.
Other boards are use it even though it's not released it

Others are good.

Thank you,
Kyungmin Park

Quoted 146 lines · Magnus Damm

From: Magnus Damm <[email protected]>

This patch removes the ARM dependency from the generic "onenand" platform device driver. This change makes the driver useful for other architectures as well. Needed for the SuperH kfr2r09 board.

Apart from the obvious Kconfig bits, the most important change is the move away from ARM specific includes and platform data. Together with this change the only in-tree board code gets an update, and the driver name is also changed gracefully break potential out of tree drivers.

The driver is also updated to allow NULL as platform data together with a few changes to make use of resource_size() and dev_name().

---

 Tested on the sh7724 kfr2r09 board with a separate platform data patch.

 arch/arm/mach-omap2/board-apollon.c |    4 ++-- drivers/mtd/onenand/Kconfig         |    1 - drivers/mtd/onenand/generic.c       |   24 ++++++++++++++---------- include/linux/mtd/onenand.h         |    8 ++++++++

 4 files changed, 24 insertions(+), 13 deletions(-)
arch/arm/mach-omap2/board-apollon.c    2009-08-04 17:01:18.000000000 +0900 renamed from arch/arm/mach-omap2/board-apollon.c +2 -2
@@ -87,7 +87,7 @@ static struct mtd_partition apollon_part
        },
  };
 
-static struct flash_platform_data apollon_flash_data = {
+static struct onenand_platform_data apollon_flash_data = {
        .parts          = apollon_partitions,
        .nr_parts       = ARRAY_SIZE(apollon_partitions),
  };
@@ -99,7 +99,7 @@ static struct resource apollon_flash_res
  };
 
  static struct platform_device apollon_onenand_device = {
-       .name           = "onenand",
+       .name           = "onenand-flash",
        .id             = -1,
        .dev            = {
                .platform_data  = &apollon_flash_data,
drivers/mtd/onenand/Kconfig    2009-08-04 17:01:18.000000000 +0900 renamed from drivers/mtd/onenand/Kconfig +0 -0
@@ -23,7 +23,6 @@ config MTD_ONENAND_VERIFY_WRITE
 

 config MTD_ONENAND_GENERIC
       tristate "OneNAND Flash device via platform device driver"
-       depends on ARM
       help
         Support for OneNAND flash via platform device driver.

drivers/mtd/onenand/generic.c  2009-08-04 17:12:00.000000000 +0900 renamed from drivers/mtd/onenand/generic.c +13 -9
@@ -19,12 +19,16 @@
  #include <linux/mtd/mtd.h>
  #include <linux/mtd/onenand.h>
  #include <linux/mtd/partitions.h>
-
  #include <asm/io.h>
-#include <asm/mach/flash.h>
-
-#define DRIVER_NAME    "onenand"
 
+/*
+ * Note: Driver name and platform data format have been updated!
+ *
+ * This version of the driver is named "onenand-flash" and takes struct
+ * onenand_platform_data as platform data. The old ARM-specific version
+ * with the name "onenand" used to take struct flash_platform_data.
+ */
+#define DRIVER_NAME    "onenand-flash"
 
  #ifdef CONFIG_MTD_PARTITIONS
  static const char *part_probes[] = { "cmdlinepart", NULL,  };
@@ -39,16 +43,16 @@ struct onenand_info {
  static int __devinit generic_onenand_probe(struct platform_device *pdev)
  {
        struct onenand_info *info;
-       struct flash_platform_data *pdata = pdev->dev.platform_data;
+       struct onenand_platform_data *pdata = pdev->dev.platform_data;
        struct resource *res = pdev->resource;
-       unsigned long size = res->end - res->start + 1;
+       unsigned long size = resource_size(res);
        int err;
 
        info = kzalloc(sizeof(struct onenand_info), GFP_KERNEL);
        if (!info)
                return -ENOMEM;
 
-       if (!request_mem_region(res->start, size, pdev->dev.driver->name)) {
+       if (!request_mem_region(res->start, size, dev_name(&pdev->dev))) {
                err = -EBUSY;
                goto out_free_info;
        }
@@ -59,7 +63,7 @@ static int __devinit generic_onenand_pro
                goto out_release_mem_region;
        }
 
-       info->onenand.mmcontrol = pdata->mmcontrol;
+       info->onenand.mmcontrol = pdata ? pdata->mmcontrol : 0;
        info->onenand.irq = platform_get_irq(pdev, 0);
 
        info->mtd.name = dev_name(&pdev->dev);
@@ -75,7 +79,7 @@ static int __devinit generic_onenand_pro
        err = parse_mtd_partitions(&info->mtd, part_probes, &info->parts, 0);
        if (err > 0)
                add_mtd_partitions(&info->mtd, info->parts, err);
-       else if (err <= 0 && pdata->parts)
+       else if (err <= 0 && pdata && pdata->parts)
                add_mtd_partitions(&info->mtd, pdata->parts, pdata->nr_parts);

       else
 #endif

drivers/mtd/onenand/generic.c  2009-08-04 17:12:00.000000000 +0900 renamed from drivers/mtd/onenand/generic.c +1 -1
@@ -99,7 +103,7 @@ static int __devexit generic_onenand_rem
  {
        struct onenand_info *info = platform_get_drvdata(pdev);
        struct resource *res = pdev->resource;
-       unsigned long size = res->end - res->start + 1;
+       unsigned long size = resource_size(res);
 
        platform_set_drvdata(pdev, NULL);
 
include/linux/mtd/onenand.h    2009-08-04 17:01:18.000000000 +0900 renamed from include/linux/mtd/onenand.h +8 -1
@@ -214,4 +214,12 @@ unsigned onenand_block(struct onenand_ch
  loff_t onenand_addr(struct onenand_chip *this, int block);
  int flexonenand_region(struct mtd_info *mtd, loff_t addr);
 
+struct mtd_partition;
+
+struct onenand_platform_data {
+       void            (*mmcontrol)(struct mtd_info *mtd, int sync_read);
+       struct mtd_partition *parts;
+       unsigned int    nr_parts;
+};
+
  #endif /* __LINUX_MTD_ONENAND_H */
--

To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

#2 lore raw

Hi Kyungmin!

Quoted 6 lines · Magnus Damm
Quoted 5 lines

From: Magnus Damm <[email protected]>

This patch removes the ARM dependency from the generic "onenand" platform device driver. This change makes the driver useful for other architectures as well. Needed for the SuperH kfr2r09 board.

Quoted 3 lines · Kyungmin Park

Good idea add the onenand_platform_data, but dont' agree the renaming the onenand-flash.
Other boards are use it even though it's not released it

I suspected so. But this is the only reason why changed the name. =)

If we don't change the name then the platform device will be attached to the platform driver as usual, but the old platform data structure will use a different binary format compated to what the driver expects.

Changing the driver name makes sure that the old device disappears and that people can move over to the new name and at the same time update the platform data to the new format.

Quoted 1 line

Others are good.

Thanks.

I can post an updated version where I keep the driver name unchanged if you prefer that. But I'm pretty sure out-of-tree drivers will break with NULL pointer accesses or similar if we update the platform data format without changing the name. I don't think you want that. =)

Cheers,

/ magnus

#3 lore raw
Quoted 23 lines · Magnus Damm
Quoted 6 lines · Magnus Damm
Quoted 5 lines

From: Magnus Damm <[email protected]>

This patch removes the ARM dependency from the generic "onenand" platform device driver. This change makes the driver useful for other architectures as well. Needed for the SuperH kfr2r09 board.

Quoted 3 lines · Kyungmin Park

Good idea add the onenand_platform_data, but dont' agree the renaming the onenand-flash.
Other boards are use it even though it's not released it

I suspected so. But this is the only reason why changed the name. =)

If we don't change the name then the platform device will be attached to the platform driver as usual, but the old platform data structure will use a different binary format compated to what the driver expects.

Changing the driver name makes sure that the old device disappears and that people can move over to the new name and at the same time update the platform data to the new format.

Quoted 8 lines
Quoted 1 line

Others are good.

Thanks.

I can post an updated version where I keep the driver name unchanged if you prefer that. But I'm pretty sure out-of-tree drivers will break with NULL pointer accesses or similar if we update the platform data format without changing the name. I don't think you want that. =)

Any update on this?

I posted a platform device patch for the kfr2r09 board to the linux-sh list yesterday, so with that platform device patch and this driver patch all in-tree users are converted.

Thanks in advance,

/ magnus

#4 lore raw
Quoted 26 lines · Magnus Damm
Quoted 25 lines · Magnus Damm
Quoted 6 lines · Magnus Damm
Quoted 5 lines

From: Magnus Damm <[email protected]>

This patch removes the ARM dependency from the generic "onenand" platform device driver. This change makes the driver useful for other architectures as well. Needed for the SuperH kfr2r09 board.

Quoted 3 lines · Kyungmin Park

Good idea add the onenand_platform_data, but dont' agree the renaming the onenand-flash.
Other boards are use it even though it's not released it

I suspected so. But this is the only reason why changed the name. =)

If we don't change the name then the platform device will be attached to the platform driver as usual, but the old platform data structure will use a different binary format compated to what the driver expects.

Changing the driver name makes sure that the old device disappears and that people can move over to the new name and at the same time update the platform data to the new format.

Quoted 1 line

Others are good.

Quoted 9 lines
Quoted 6 lines

Thanks.

I can post an updated version where I keep the driver name unchanged if you prefer that. But I'm pretty sure out-of-tree drivers will break with NULL pointer accesses or similar if we update the platform data format without changing the name. I don't think you want that. =)

Any update on this?

You have failed at reading your email:

http://lists.infradead.org/pipermail/linux-mtd/2009-August/026805.html
http://lists.infradead.org/pipermail/linux-mtd/2009-August/026809.html

#5 lore raw
Quoted 9 lines · Paul Mundt
Quoted 8 lines · Magnus Damm
Quoted 7 lines · Magnus Damm
Quoted 6 lines · Magnus Damm
Quoted 5 lines

From: Magnus Damm <[email protected]>

This patch removes the ARM dependency from the generic "onenand" platform device driver. This change makes the driver useful for other architectures as well. Needed for the SuperH kfr2r09 board.

Quoted 6 lines
Quoted 2 lines

Any update on this?

You have failed at reading your email:

http://lists.infradead.org/pipermail/linux-mtd/2009-August/026805.html
http://lists.infradead.org/pipermail/linux-mtd/2009-August/026809.html

As usual, sorry about that. I'll solve that soon enough by signing off with a different address.

/ magnus

#6 lore raw
In reply to Paul Mundt
Quoted 9 lines · Paul Mundt
Quoted 8 lines · Magnus Damm
Quoted 7 lines · Magnus Damm
Quoted 6 lines · Magnus Damm
Quoted 5 lines

From: Magnus Damm <[email protected]>

This patch removes the ARM dependency from the generic "onenand" platform device driver. This change makes the driver useful for other architectures as well. Needed for the SuperH kfr2r09 board.

Quoted 6 lines
Quoted 2 lines

Any update on this?

You have failed at reading your email:

http://lists.infradead.org/pipermail/linux-mtd/2009-August/026805.html
http://lists.infradead.org/pipermail/linux-mtd/2009-August/026809.html

As usual, sorry about that. I'll solve that soon enough by signing off with a different address.

/ magnus