All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] mtdpart: memory accessor interface for MTD layer
@ 2010-03-16 20:41 ` Kevin Hilman
  0 siblings, 0 replies; 29+ messages in thread
From: Kevin Hilman @ 2010-03-16 20:41 UTC (permalink / raw)
  To: linux-mtd
  Cc: Sudhakar Rajashekhara, David Brownell, David Woodhouse,
	Andrew Morton, Bernd Schmidt, David Howells, David Brownell,
	Nicolas Pitre, Kevin Hilman, linux-kernel

From: Sudhakar Rajashekhara <sudhakar....@ti.com>

This patch implements memory accessor interface in the MTD
layer which enables the kernel to access flash data.

This patch adds two new members to the mtd_partition structure,
a function handler which will be called during setup of the
partition and an argument to be passed to this setup function.

Example:
+static struct mtd_partition spi_flash_partitions[] = {
+       [0] = {
+               .name       = "U-Boot",
+               .offset     = 0,
+               .size       = SZ_256K,
+               .mask_flags = MTD_WRITEABLE,
+       },
+       [1] = {
+               .name       = "U-Boot Environment",
+               .offset     = MTDPART_OFS_NXTBLK,
+               .size       = SZ_64K,
+               .mask_flags = MTD_WRITEABLE,
+       },
+       [2] = {
+               .name       = "Linux",
+               .offset     = MTDPART_OFS_NXTBLK,
+               .size       = SZ_7M,
+               .mask_flags = 0,
+       },
+       [3] = {
+               .name       = "MAC Address",
+               .offset     = MTDPART_OFS_NXTBLK,
+               .size       = SZ_64K,
+               .mask_flags = 0,
+               .setup      = davinci_get_mac_addr,
+               .context    = (void *)0,
+       },
+};

The davinci_get_mac_addr function reads the MAC address from
offset ZERO of last MTD partition.

Signed-off-by: Sudhakar Rajashekhara <sudhakar.raj@ti.com>
Acked-by: Kevin Hilman <khilman@deeprootsystems.com>
Cc: David Brownell <david-b@pacbell.net>
Cc: David Woodhouse <dwmw2@infradead.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
I thought this had made it to -mm, but was dropped waiting
for subsystem maintainer to pick it up.  Here it is againg for 2.6.35.

 drivers/mtd/mtdpart.c          |   40 ++++++++++++++++++++++++++++++++++++++++
 include/linux/mtd/partitions.h |    3 +++
 2 files changed, 43 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index b8043a9..9f4d5f8 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -26,6 +26,7 @@ static LIST_HEAD(mtd_partitions);
 struct mtd_part {
 	struct mtd_info mtd;
 	struct mtd_info *master;
+	struct memory_accessor macc;
 	uint64_t offset;
 	struct list_head list;
 };
@@ -327,6 +328,39 @@ int del_mtd_partitions(struct mtd_info *master)
 }
 EXPORT_SYMBOL(del_mtd_partitions);
 
+/*
+ * This lets other kernel code access the flash data. For example, it
+ * might hold a board's Ethernet address, or board-specific calibration
+ * data generated on the manufacturing floor.
+ */
+static ssize_t mtd_macc_read(struct memory_accessor *macc, char *buf,
+			off_t offset, size_t count)
+{
+	struct mtd_part *part = container_of(macc, struct mtd_part, macc);
+	ssize_t ret = -EIO;
+	size_t retlen;
+
+	if (part_read((struct mtd_info *)part, offset, count,
+			&retlen, buf) == 0)
+		ret = retlen;
+
+	return ret;
+}
+
+static ssize_t mtd_macc_write(struct memory_accessor *macc, const char *buf,
+			off_t offset, size_t count)
+{
+	struct mtd_part *part = container_of(macc, struct mtd_part, macc);
+	ssize_t ret = -EIO;
+	size_t retlen;
+
+	if (part_write((struct mtd_info *)part, offset, count,
+			&retlen, buf) == 0)
+		ret = retlen;
+
+	return ret;
+}
+
 static struct mtd_part *add_one_partition(struct mtd_info *master,
 		const struct mtd_partition *part, int partno,
 		uint64_t cur_offset)
@@ -364,6 +398,9 @@ static struct mtd_part *add_one_partition(struct mtd_info *master,
 	slave->mtd.read = part_read;
 	slave->mtd.write = part_write;
 
+	slave->macc.read = mtd_macc_read;
+	slave->macc.write = mtd_macc_write;
+
 	if (master->panic_write)
 		slave->mtd.panic_write = part_panic_write;
 
@@ -428,6 +465,9 @@ static struct mtd_part *add_one_partition(struct mtd_info *master,
 	printk(KERN_NOTICE "0x%012llx-0x%012llx : \"%s\"\n", (unsigned long long)slave->offset,
 		(unsigned long long)(slave->offset + slave->mtd.size), slave->mtd.name);
 
+	if (part->setup)
+		part->setup(&slave->macc, (void *)part->context);
+
 	/* let's do some sanity checks */
 	if (slave->offset >= master->size) {
 		/* let's register it anyway to preserve ordering */
diff --git a/include/linux/mtd/partitions.h b/include/linux/mtd/partitions.h
index 274b619..39782a6 100644
--- a/include/linux/mtd/partitions.h
+++ b/include/linux/mtd/partitions.h
@@ -10,6 +10,7 @@
 #define MTD_PARTITIONS_H
 
 #include <linux/types.h>
+#include <linux/memory.h>
 
 
 /*
@@ -40,6 +41,8 @@ struct mtd_partition {
 	uint64_t offset;		/* offset within the master MTD space */
 	uint32_t mask_flags;		/* master MTD flags to mask out for this partition */
 	struct nand_ecclayout *ecclayout;	/* out of band layout for this partition (NAND only)*/
+	void (*setup)(struct memory_accessor *, void *context);
+	void *context;
 };
 
 #define MTDPART_OFS_NXTBLK	(-2)
-- 
1.7.0.2


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

* [PATCH 1/2] mtdpart: memory accessor interface for MTD layer
@ 2010-03-16 20:41 ` Kevin Hilman
  0 siblings, 0 replies; 29+ messages in thread
From: Kevin Hilman @ 2010-03-16 20:41 UTC (permalink / raw)
  To: linux-mtd
  Cc: Bernd Schmidt, David Howells, Sudhakar Rajashekhara,
	Nicolas Pitre, Kevin Hilman, linux-kernel, David Brownell,
	David Brownell, Andrew Morton, David Woodhouse

From: Sudhakar Rajashekhara <sudhakar....@ti.com>

This patch implements memory accessor interface in the MTD
layer which enables the kernel to access flash data.

This patch adds two new members to the mtd_partition structure,
a function handler which will be called during setup of the
partition and an argument to be passed to this setup function.

Example:
+static struct mtd_partition spi_flash_partitions[] = {
+       [0] = {
+               .name       = "U-Boot",
+               .offset     = 0,
+               .size       = SZ_256K,
+               .mask_flags = MTD_WRITEABLE,
+       },
+       [1] = {
+               .name       = "U-Boot Environment",
+               .offset     = MTDPART_OFS_NXTBLK,
+               .size       = SZ_64K,
+               .mask_flags = MTD_WRITEABLE,
+       },
+       [2] = {
+               .name       = "Linux",
+               .offset     = MTDPART_OFS_NXTBLK,
+               .size       = SZ_7M,
+               .mask_flags = 0,
+       },
+       [3] = {
+               .name       = "MAC Address",
+               .offset     = MTDPART_OFS_NXTBLK,
+               .size       = SZ_64K,
+               .mask_flags = 0,
+               .setup      = davinci_get_mac_addr,
+               .context    = (void *)0,
+       },
+};

The davinci_get_mac_addr function reads the MAC address from
offset ZERO of last MTD partition.

Signed-off-by: Sudhakar Rajashekhara <sudhakar.raj@ti.com>
Acked-by: Kevin Hilman <khilman@deeprootsystems.com>
Cc: David Brownell <david-b@pacbell.net>
Cc: David Woodhouse <dwmw2@infradead.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
I thought this had made it to -mm, but was dropped waiting
for subsystem maintainer to pick it up.  Here it is againg for 2.6.35.

 drivers/mtd/mtdpart.c          |   40 ++++++++++++++++++++++++++++++++++++++++
 include/linux/mtd/partitions.h |    3 +++
 2 files changed, 43 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index b8043a9..9f4d5f8 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -26,6 +26,7 @@ static LIST_HEAD(mtd_partitions);
 struct mtd_part {
 	struct mtd_info mtd;
 	struct mtd_info *master;
+	struct memory_accessor macc;
 	uint64_t offset;
 	struct list_head list;
 };
@@ -327,6 +328,39 @@ int del_mtd_partitions(struct mtd_info *master)
 }
 EXPORT_SYMBOL(del_mtd_partitions);
 
+/*
+ * This lets other kernel code access the flash data. For example, it
+ * might hold a board's Ethernet address, or board-specific calibration
+ * data generated on the manufacturing floor.
+ */
+static ssize_t mtd_macc_read(struct memory_accessor *macc, char *buf,
+			off_t offset, size_t count)
+{
+	struct mtd_part *part = container_of(macc, struct mtd_part, macc);
+	ssize_t ret = -EIO;
+	size_t retlen;
+
+	if (part_read((struct mtd_info *)part, offset, count,
+			&retlen, buf) == 0)
+		ret = retlen;
+
+	return ret;
+}
+
+static ssize_t mtd_macc_write(struct memory_accessor *macc, const char *buf,
+			off_t offset, size_t count)
+{
+	struct mtd_part *part = container_of(macc, struct mtd_part, macc);
+	ssize_t ret = -EIO;
+	size_t retlen;
+
+	if (part_write((struct mtd_info *)part, offset, count,
+			&retlen, buf) == 0)
+		ret = retlen;
+
+	return ret;
+}
+
 static struct mtd_part *add_one_partition(struct mtd_info *master,
 		const struct mtd_partition *part, int partno,
 		uint64_t cur_offset)
@@ -364,6 +398,9 @@ static struct mtd_part *add_one_partition(struct mtd_info *master,
 	slave->mtd.read = part_read;
 	slave->mtd.write = part_write;
 
+	slave->macc.read = mtd_macc_read;
+	slave->macc.write = mtd_macc_write;
+
 	if (master->panic_write)
 		slave->mtd.panic_write = part_panic_write;
 
@@ -428,6 +465,9 @@ static struct mtd_part *add_one_partition(struct mtd_info *master,
 	printk(KERN_NOTICE "0x%012llx-0x%012llx : \"%s\"\n", (unsigned long long)slave->offset,
 		(unsigned long long)(slave->offset + slave->mtd.size), slave->mtd.name);
 
+	if (part->setup)
+		part->setup(&slave->macc, (void *)part->context);
+
 	/* let's do some sanity checks */
 	if (slave->offset >= master->size) {
 		/* let's register it anyway to preserve ordering */
diff --git a/include/linux/mtd/partitions.h b/include/linux/mtd/partitions.h
index 274b619..39782a6 100644
--- a/include/linux/mtd/partitions.h
+++ b/include/linux/mtd/partitions.h
@@ -10,6 +10,7 @@
 #define MTD_PARTITIONS_H
 
 #include <linux/types.h>
+#include <linux/memory.h>
 
 
 /*
@@ -40,6 +41,8 @@ struct mtd_partition {
 	uint64_t offset;		/* offset within the master MTD space */
 	uint32_t mask_flags;		/* master MTD flags to mask out for this partition */
 	struct nand_ecclayout *ecclayout;	/* out of band layout for this partition (NAND only)*/
+	void (*setup)(struct memory_accessor *, void *context);
+	void *context;
 };
 
 #define MTDPART_OFS_NXTBLK	(-2)
-- 
1.7.0.2

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

* Re: [PATCH 1/2] mtdpart: memory accessor interface for MTD layer
  2010-03-16 20:41 ` Kevin Hilman
@ 2010-04-08  8:10   ` Artem Bityutskiy
  -1 siblings, 0 replies; 29+ messages in thread
From: Artem Bityutskiy @ 2010-04-08  8:10 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: linux-mtd, Sudhakar Rajashekhara, David Brownell,
	David Woodhouse, Andrew Morton, Bernd Schmidt, David Howells,
	David Brownell, Nicolas Pitre, linux-kernel

On Tue, 2010-03-16 at 13:41 -0700, Kevin Hilman wrote:
> From: Sudhakar Rajashekhara <sudhakar....@ti.com>
> 
> This patch implements memory accessor interface in the MTD
> layer which enables the kernel to access flash data.
> 
> This patch adds two new members to the mtd_partition structure,
> a function handler which will be called during setup of the
> partition and an argument to be passed to this setup function.

I've pushed this one to my l2-mtd-2.6.git tree, branch "dunno".

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)


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

* Re: [PATCH 1/2] mtdpart: memory accessor interface for MTD layer
@ 2010-04-08  8:10   ` Artem Bityutskiy
  0 siblings, 0 replies; 29+ messages in thread
From: Artem Bityutskiy @ 2010-04-08  8:10 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: Bernd Schmidt, David Howells, Sudhakar Rajashekhara,
	Nicolas Pitre, linux-kernel, David Brownell, David Brownell,
	linux-mtd, Andrew Morton, David Woodhouse

On Tue, 2010-03-16 at 13:41 -0700, Kevin Hilman wrote:
> From: Sudhakar Rajashekhara <sudhakar....@ti.com>
> 
> This patch implements memory accessor interface in the MTD
> layer which enables the kernel to access flash data.
> 
> This patch adds two new members to the mtd_partition structure,
> a function handler which will be called during setup of the
> partition and an argument to be passed to this setup function.

I've pushed this one to my l2-mtd-2.6.git tree, branch "dunno".

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

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

* Re: [PATCH 1/2] mtdpart: memory accessor interface for MTD layer
  2010-03-16 20:41 ` Kevin Hilman
@ 2010-05-13 23:50   ` David Woodhouse
  -1 siblings, 0 replies; 29+ messages in thread
From: David Woodhouse @ 2010-05-13 23:50 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: linux-mtd, Bernd Schmidt, David Howells, Sudhakar Rajashekhara,
	Nicolas Pitre, linux-kernel, David Brownell, David Brownell,
	Andrew Morton

On Tue, 2010-03-16 at 13:41 -0700, Kevin Hilman wrote:
> From: Sudhakar Rajashekhara <sudhakar....@ti.com>
> 
> This patch implements memory accessor interface in the MTD
> layer which enables the kernel to access flash data.
> 
> This patch adds two new members to the mtd_partition structure,
> a function handler which will be called during setup of the
> partition and an argument to be passed to this setup function.

Ick.

I don't mind providing the mtd_macc_{read,write} helper functions,
although they should work on generic MTD devices not just on partitions.
But do we really have to do the callout to arbitrary functions from
_within_ the core MTD code... why can't we do that in the board driver?

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation


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

* Re: [PATCH 1/2] mtdpart: memory accessor interface for MTD layer
@ 2010-05-13 23:50   ` David Woodhouse
  0 siblings, 0 replies; 29+ messages in thread
From: David Woodhouse @ 2010-05-13 23:50 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: Bernd Schmidt, David Brownell, Sudhakar Rajashekhara,
	Nicolas Pitre, linux-kernel, David Howells, David Brownell,
	linux-mtd, Andrew Morton

On Tue, 2010-03-16 at 13:41 -0700, Kevin Hilman wrote:
> From: Sudhakar Rajashekhara <sudhakar....@ti.com>
> 
> This patch implements memory accessor interface in the MTD
> layer which enables the kernel to access flash data.
> 
> This patch adds two new members to the mtd_partition structure,
> a function handler which will be called during setup of the
> partition and an argument to be passed to this setup function.

Ick.

I don't mind providing the mtd_macc_{read,write} helper functions,
although they should work on generic MTD devices not just on partitions.
But do we really have to do the callout to arbitrary functions from
_within_ the core MTD code... why can't we do that in the board driver?

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

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

* RE: [PATCH 1/2] mtdpart: memory accessor interface for MTD layer
  2010-05-13 23:50   ` David Woodhouse
  (?)
@ 2010-07-07 10:56   ` Sudhakar Rajashekhara
  2010-07-07 11:08       ` David Brownell
  -1 siblings, 1 reply; 29+ messages in thread
From: Sudhakar Rajashekhara @ 2010-07-07 10:56 UTC (permalink / raw)
  To: 'David Woodhouse', 'Kevin Hilman'
  Cc: 'Bernd Schmidt', 'David Howells',
	'David Brownell', 'Nicolas Pitre',
	linux-kernel, 'David Brownell',
	linux-mtd, 'Andrew Morton'

Hi David,

On Fri, May 14, 2010 at 05:20:29, David Woodhouse wrote:
> On Tue, 2010-03-16 at 13:41 -0700, Kevin Hilman wrote:
> > From: Sudhakar Rajashekhara <sudhakar....@ti.com>
> > 
> > This patch implements memory accessor interface in the MTD
> > layer which enables the kernel to access flash data.
> > 
> > This patch adds two new members to the mtd_partition structure,
> > a function handler which will be called during setup of the
> > partition and an argument to be passed to this setup function.
> 
> Ick.
> 
> I don't mind providing the mtd_macc_{read,write} helper functions,
> although they should work on generic MTD devices not just on partitions.
> But do we really have to do the callout to arbitrary functions from
> _within_ the core MTD code... why can't we do that in the board driver?
> 

First of all apologies for replying to this thread *very* late.

This thread started in response to the patch at
http://lkml.org/lkml/2010/3/16/380. I think you are OK with the
mtd_macc_{read,write} functions being provided within the MTD core but
you are objecting to the extra ->setup method in struct mtd_partition.
Without adding the setup method in mtd_parition structure, the MTD
memory accessor interface users will not knowing when to call the
mtd_macc_{read,write} functions. These functions should be called after
the MTD partitions are set up. If the ->setup method is moved to the
board driver, I'll get into the initialization sequence problem, which
I want to avoid.

Thanks,
Sudhakar

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

* RE: [PATCH 1/2] mtdpart: memory accessor interface for MTD layer
  2010-07-07 10:56   ` Sudhakar Rajashekhara
@ 2010-07-07 11:08       ` David Brownell
  0 siblings, 0 replies; 29+ messages in thread
From: David Brownell @ 2010-07-07 11:08 UTC (permalink / raw)
  To: 'David Woodhouse', 'Kevin Hilman', Sudhakar Rajashekhara
  Cc: 'Bernd Schmidt', 'Nicolas Pitre',
	linux-kernel, 'David Howells', 'David Brownell',
	linux-mtd, 'Andrew Morton'


> > > This patch adds two new members to the
> mtd_partition structure,
> > > a function handler which will be
> > > called during setup of the
> > > partition and an argument to be passed to this
> setup function.
> > 
> > Ick.
> > 
> > I don't mind providing the mtd_macc_{read,write}
> helper functions,
> > although they should work on generic MTD
> > >devices not just on partitions.
> > But do we really have to do the callout to
> > arbitrary functions from
> > _within_ the core MTD code... why can't we
> > do that in the board driver?

I think the short answer is that the callout is
what provides the board drivers enough information
to make the correct calls.



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

* RE: [PATCH 1/2] mtdpart: memory accessor interface for MTD layer
@ 2010-07-07 11:08       ` David Brownell
  0 siblings, 0 replies; 29+ messages in thread
From: David Brownell @ 2010-07-07 11:08 UTC (permalink / raw)
  To: 'David Woodhouse', 'Kevin Hilman', Sudhakar Rajashekhara
  Cc: 'Bernd Schmidt', 'David Brownell',
	'Nicolas Pitre', linux-kernel, 'David Howells',
	linux-mtd, 'Andrew Morton'


> > > This patch adds two new members to the
> mtd_partition structure,
> > > a function handler which will be
> > > called during setup of the
> > > partition and an argument to be passed to this
> setup function.
> > 
> > Ick.
> > 
> > I don't mind providing the mtd_macc_{read,write}
> helper functions,
> > although they should work on generic MTD
> > >devices not just on partitions.
> > But do we really have to do the callout to
> > arbitrary functions from
> > _within_ the core MTD code... why can't we
> > do that in the board driver?

I think the short answer is that the callout is
what provides the board drivers enough information
to make the correct calls.

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

* RE: [PATCH 1/2] mtdpart: memory accessor interface for MTD layer
  2010-07-07 11:08       ` David Brownell
  (?)
@ 2010-07-08 15:10       ` Sudhakar Rajashekhara
  2010-07-08 16:00         ` David Brownell
  -1 siblings, 1 reply; 29+ messages in thread
From: Sudhakar Rajashekhara @ 2010-07-08 15:10 UTC (permalink / raw)
  To: 'David Brownell', 'David Woodhouse',
	'Kevin Hilman'
  Cc: 'Bernd Schmidt', 'David Brownell',
	'Nicolas Pitre', linux-kernel, 'David Howells',
	linux-mtd, 'Andrew Morton'

Hi Dave,

On Wed, Jul 07, 2010 at 16:38:54, David Brownell wrote:
> 
> > > > This patch adds two new members to the
> > mtd_partition structure,
> > > > a function handler which will be
> > > > called during setup of the
> > > > partition and an argument to be passed to this
> > setup function.
> > > 
> > > Ick.
> > > 
> > > I don't mind providing the mtd_macc_{read,write}
> > helper functions,
> > > although they should work on generic MTD
> > > >devices not just on partitions.
> > > But do we really have to do the callout to
> > > arbitrary functions from
> > > _within_ the core MTD code... why can't we
> > > do that in the board driver?
> 
> I think the short answer is that the callout is
> what provides the board drivers enough information
> to make the correct calls.
> 

If you agree to what the patch is doing, then can you Ack it?

Thanks,
Sudhakar

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

* RE: [PATCH 1/2] mtdpart: memory accessor interface for MTD layer
  2010-07-08 15:10       ` Sudhakar Rajashekhara
@ 2010-07-08 16:00         ` David Brownell
  0 siblings, 0 replies; 29+ messages in thread
From: David Brownell @ 2010-07-08 16:00 UTC (permalink / raw)
  To: 'David Woodhouse', 'Kevin Hilman', Sudhakar Rajashekhara
  Cc: 'Bernd Schmidt', 'David Brownell',
	'Nicolas Pitre', linux-kernel, 'David Howells',
	linux-mtd, 'Andrew Morton'


> > I think the short answer is that the callout is
> > what provides the board drivers enough information
> > to make the correct calls.
> > 
> 
> If you agree to what the patch is doing, then can you Ack it?

Iff it's resent; it's not in my mbox any more.
 


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

* RE: [PATCH 1/2] mtdpart: memory accessor interface for MTD layer
  2010-07-07 11:08       ` David Brownell
@ 2010-08-04 10:12         ` David Woodhouse
  -1 siblings, 0 replies; 29+ messages in thread
From: David Woodhouse @ 2010-08-04 10:12 UTC (permalink / raw)
  To: David Brownell
  Cc: 'Kevin Hilman',
	Sudhakar Rajashekhara, 'Bernd Schmidt',
	'Nicolas Pitre', linux-kernel, 'David Howells',
	'David Brownell', linux-mtd, 'Andrew Morton'

On Wed, 2010-07-07 at 04:08 -0700, David Brownell wrote:
> 
> I think the short answer is that the callout is
> what provides the board drivers enough information
> to make the correct calls. 

I don't see how.

The only information it passes to the callout is the information it was
already *given* in the partition structure.

I'm more inclined to believe Sudhakar's claim that you'll get an
'initialization sequence problem', although I'm not sure I believe it
can't be solved in a better way than this.

I'm also unhappy that it only works on partitioned devices -- that seems
wrong.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation



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

* RE: [PATCH 1/2] mtdpart: memory accessor interface for MTD layer
@ 2010-08-04 10:12         ` David Woodhouse
  0 siblings, 0 replies; 29+ messages in thread
From: David Woodhouse @ 2010-08-04 10:12 UTC (permalink / raw)
  To: David Brownell
  Cc: 'Bernd Schmidt',
	Sudhakar Rajashekhara, 'Nicolas Pitre',
	'Kevin Hilman', linux-kernel, 'David Howells',
	'David Brownell', linux-mtd, 'Andrew Morton'

On Wed, 2010-07-07 at 04:08 -0700, David Brownell wrote:
> 
> I think the short answer is that the callout is
> what provides the board drivers enough information
> to make the correct calls. 

I don't see how.

The only information it passes to the callout is the information it was
already *given* in the partition structure.

I'm more inclined to believe Sudhakar's claim that you'll get an
'initialization sequence problem', although I'm not sure I believe it
can't be solved in a better way than this.

I'm also unhappy that it only works on partitioned devices -- that seems
wrong.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

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

* RE: [PATCH 1/2] mtdpart: memory accessor interface for MTD layer
  2010-08-04 10:12         ` David Woodhouse
@ 2010-08-04 10:31           ` David Brownell
  -1 siblings, 0 replies; 29+ messages in thread
From: David Brownell @ 2010-08-04 10:31 UTC (permalink / raw)
  To: David Woodhouse
  Cc: 'Kevin Hilman',
	Sudhakar Rajashekhara, 'Bernd Schmidt',
	'Nicolas Pitre', linux-kernel, 'David Howells',
	'David Brownell', linux-mtd, 'Andrew Morton'



--- On Wed, 8/4/10, David Woodhouse <dwmw2@infradead.org> wrote:
> On Wed, 2010-07-07 at 04:08 -0700,
> David Brownell wrote:
> > 
> > I think the short answer is that the callout is
> > what provides the board drivers enough information
> > to make the correct calls. 

EMPHASIS ON "CORRECT" ...

> I don't see how.
> 
> The only information it passes to the callout is the
> information it was
> already *given* in the partition structure.

There can be multiple such structures, each
of which describes different data to be extracted
from different persistent media contexts.

Examples:  one context holds one MAC address (and
another, a different one) ... another might embed
calibration data; another, serial numbers; etc.

Pass the wrong context around, you've trashed all
the data instead of getting it right.


> I'm more inclined to believe Sudhakar's claim that you'll
> get an
> 'initialization sequence problem', although I'm not sure I
> believe it
> can't be solved in a better way than this.

"Initialization sequence" is a grab-bag category
that covers most init issues.

Point is to ensure that enough of the right context
information is available to initialize correctly.
So the right data is extracted and passed on.


> 
> I'm also unhappy that it only works on partitioned devices
> -- that seems wrong.

Very different issue.  Seems easily fixable
if needed.  Agreed that e.g. EEPROMS won't
often be partitioned (unlike flash).




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

* RE: [PATCH 1/2] mtdpart: memory accessor interface for MTD layer
@ 2010-08-04 10:31           ` David Brownell
  0 siblings, 0 replies; 29+ messages in thread
From: David Brownell @ 2010-08-04 10:31 UTC (permalink / raw)
  To: David Woodhouse
  Cc: 'Bernd Schmidt',
	Sudhakar Rajashekhara, 'Nicolas Pitre',
	'Kevin Hilman', linux-kernel, 'David Howells',
	'David Brownell', linux-mtd, 'Andrew Morton'



--- On Wed, 8/4/10, David Woodhouse <dwmw2@infradead.org> wrote:
> On Wed, 2010-07-07 at 04:08 -0700,
> David Brownell wrote:
> > 
> > I think the short answer is that the callout is
> > what provides the board drivers enough information
> > to make the correct calls. 

EMPHASIS ON "CORRECT" ...

> I don't see how.
> 
> The only information it passes to the callout is the
> information it was
> already *given* in the partition structure.

There can be multiple such structures, each
of which describes different data to be extracted
from different persistent media contexts.

Examples:  one context holds one MAC address (and
another, a different one) ... another might embed
calibration data; another, serial numbers; etc.

Pass the wrong context around, you've trashed all
the data instead of getting it right.


> I'm more inclined to believe Sudhakar's claim that you'll
> get an
> 'initialization sequence problem', although I'm not sure I
> believe it
> can't be solved in a better way than this.

"Initialization sequence" is a grab-bag category
that covers most init issues.

Point is to ensure that enough of the right context
information is available to initialize correctly.
So the right data is extracted and passed on.


> 
> I'm also unhappy that it only works on partitioned devices
> -- that seems wrong.

Very different issue.  Seems easily fixable
if needed.  Agreed that e.g. EEPROMS won't
often be partitioned (unlike flash).

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

* RE: [PATCH 1/2] mtdpart: memory accessor interface for MTD layer
  2010-08-04 10:31           ` David Brownell
@ 2010-08-04 11:08             ` David Woodhouse
  -1 siblings, 0 replies; 29+ messages in thread
From: David Woodhouse @ 2010-08-04 11:08 UTC (permalink / raw)
  To: David Brownell
  Cc: 'Kevin Hilman',
	Sudhakar Rajashekhara, 'Bernd Schmidt',
	'Nicolas Pitre', linux-kernel, 'David Howells',
	'David Brownell', linux-mtd, 'Andrew Morton'

On Wed, 2010-08-04 at 03:31 -0700, David Brownell wrote:
> Point is to ensure that enough of the right context
> information is available to initialize correctly.
> So the right data is extracted and passed on.

Forgive me if I'm being dim (and in particular, please forgive me if I'm
going over something that was already discussed; I know it's been a
while). But I don't see why it needs to be passed through the core MTD
code.

To take the simple case of an unpartitioned MTD device -- why can't the
map driver (or whatever) just call the maccessor setup function for
itself, directly, right after calling add_mtd_device() with its
newly-probed MTD device?

And for partitions, why can't it do the same, on the appropriate
partition.

OK, the answer to the latter question is that you don't actually *have*
the pointers to each partition you register. But that's easily fixed.

If we make add_mtd_partitions() take an extra 'struct mtd_info **'
argument and put pointers to the slave mtd 'devices' into that, it means
that your board driver *can* reliably get the mtd pointer for the fourth
partition, or whatever it needs. And can then just do the memory
accessor setup for itself.

Isn't that enough?

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation



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

* RE: [PATCH 1/2] mtdpart: memory accessor interface for MTD layer
@ 2010-08-04 11:08             ` David Woodhouse
  0 siblings, 0 replies; 29+ messages in thread
From: David Woodhouse @ 2010-08-04 11:08 UTC (permalink / raw)
  To: David Brownell
  Cc: 'Bernd Schmidt',
	Sudhakar Rajashekhara, 'Nicolas Pitre',
	'Kevin Hilman', linux-kernel, 'David Howells',
	'David Brownell', linux-mtd, 'Andrew Morton'

On Wed, 2010-08-04 at 03:31 -0700, David Brownell wrote:
> Point is to ensure that enough of the right context
> information is available to initialize correctly.
> So the right data is extracted and passed on.

Forgive me if I'm being dim (and in particular, please forgive me if I'm
going over something that was already discussed; I know it's been a
while). But I don't see why it needs to be passed through the core MTD
code.

To take the simple case of an unpartitioned MTD device -- why can't the
map driver (or whatever) just call the maccessor setup function for
itself, directly, right after calling add_mtd_device() with its
newly-probed MTD device?

And for partitions, why can't it do the same, on the appropriate
partition.

OK, the answer to the latter question is that you don't actually *have*
the pointers to each partition you register. But that's easily fixed.

If we make add_mtd_partitions() take an extra 'struct mtd_info **'
argument and put pointers to the slave mtd 'devices' into that, it means
that your board driver *can* reliably get the mtd pointer for the fourth
partition, or whatever it needs. And can then just do the memory
accessor setup for itself.

Isn't that enough?

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

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

* RE: [PATCH 1/2] mtdpart: memory accessor interface for MTD layer
  2010-08-04 11:08             ` David Woodhouse
@ 2010-08-04 11:27               ` David Brownell
  -1 siblings, 0 replies; 29+ messages in thread
From: David Brownell @ 2010-08-04 11:27 UTC (permalink / raw)
  To: David Woodhouse
  Cc: 'Kevin Hilman',
	Sudhakar Rajashekhara, 'Bernd Schmidt',
	'Nicolas Pitre', linux-kernel, 'David Howells',
	'David Brownell', linux-mtd, 'Andrew Morton'



--- On Wed, 8/4/10, David Woodhouse <dwmw2@infradead.org> wrote:

> > Point is to ensure that enough of the right context
> > information is available to initialize correctly.
> > So the right data is extracted and passed on.


And also, ISTR, that the mechanism is general
enough to work with both MTD and EEPROM ...

> 
> Forgive me if I'm being dim (and in particular, please
> forgive me if I'm
> going over something that was already discussed; I know
> it's been a
> while).

I also am at risk of getting lost in a pile
of hypotheticals which have been left behind
earlier in these threads.

 But I don't see why it needs to be passed through
> the core MTD  code.
> 
> To take the simple case of an unpartitioned MTD device --
> why can't the
> map driver (or whatever) just call the maccessor setup
> function for
> itself, directly, right after calling add_mtd_device() with
> its newly-probed MTD device?

No idea, except that doing it once rather than
modifying every driver would seem healthier.
Surely changing all drivers is a Bad Thing.

> 
> And for partitions, why can't it do the same, on the
> appropriate partition.
> 
> OK, the answer to the latter question is that you don't
> actually *have*
> the pointers to each partition you register. But that's
> easily fixed.
> 
> If we make add_mtd_partitions() take an extra 'struct
> mtd_info **'
> argument and put pointers to the slave mtd 'devices' into
> that, it means
> that your board driver *can* reliably get the mtd pointer
> for the fourth
> partition, or whatever it needs. And can then just do the
> memory
> accessor setup for itself.
> 
> Isn't that enough?

Might be.  Not my patch though...  You asked why
the context was needed along with the partition
data (otherwise not available); I answered.

Still haven't seen a better patch though.






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

* RE: [PATCH 1/2] mtdpart: memory accessor interface for MTD layer
@ 2010-08-04 11:27               ` David Brownell
  0 siblings, 0 replies; 29+ messages in thread
From: David Brownell @ 2010-08-04 11:27 UTC (permalink / raw)
  To: David Woodhouse
  Cc: 'Bernd Schmidt',
	Sudhakar Rajashekhara, 'Nicolas Pitre',
	'Kevin Hilman', linux-kernel, 'David Howells',
	'David Brownell', linux-mtd, 'Andrew Morton'



--- On Wed, 8/4/10, David Woodhouse <dwmw2@infradead.org> wrote:

> > Point is to ensure that enough of the right context
> > information is available to initialize correctly.
> > So the right data is extracted and passed on.


And also, ISTR, that the mechanism is general
enough to work with both MTD and EEPROM ...

> 
> Forgive me if I'm being dim (and in particular, please
> forgive me if I'm
> going over something that was already discussed; I know
> it's been a
> while).

I also am at risk of getting lost in a pile
of hypotheticals which have been left behind
earlier in these threads.

 But I don't see why it needs to be passed through
> the core MTD  code.
> 
> To take the simple case of an unpartitioned MTD device --
> why can't the
> map driver (or whatever) just call the maccessor setup
> function for
> itself, directly, right after calling add_mtd_device() with
> its newly-probed MTD device?

No idea, except that doing it once rather than
modifying every driver would seem healthier.
Surely changing all drivers is a Bad Thing.

> 
> And for partitions, why can't it do the same, on the
> appropriate partition.
> 
> OK, the answer to the latter question is that you don't
> actually *have*
> the pointers to each partition you register. But that's
> easily fixed.
> 
> If we make add_mtd_partitions() take an extra 'struct
> mtd_info **'
> argument and put pointers to the slave mtd 'devices' into
> that, it means
> that your board driver *can* reliably get the mtd pointer
> for the fourth
> partition, or whatever it needs. And can then just do the
> memory
> accessor setup for itself.
> 
> Isn't that enough?

Might be.  Not my patch though...  You asked why
the context was needed along with the partition
data (otherwise not available); I answered.

Still haven't seen a better patch though.

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

* RE: [PATCH 1/2] mtdpart: memory accessor interface for MTD layer
  2010-08-04 11:08             ` David Woodhouse
  (?)
  (?)
@ 2010-08-06  6:48             ` Sudhakar Rajashekhara
  2010-08-08 12:23                 ` David Woodhouse
  -1 siblings, 1 reply; 29+ messages in thread
From: Sudhakar Rajashekhara @ 2010-08-06  6:48 UTC (permalink / raw)
  To: 'David Woodhouse', 'David Brownell'
  Cc: 'Bernd Schmidt', 'David Brownell',
	'Nicolas Pitre', 'Kevin Hilman',
	linux-kernel, 'David Howells',
	linux-mtd, 'Andrew Morton'

Hi,

On Wed, Aug 04, 2010 at 16:38:47, David Woodhouse wrote:
> On Wed, 2010-08-04 at 03:31 -0700, David Brownell wrote:
> > Point is to ensure that enough of the right context
> > information is available to initialize correctly.
> > So the right data is extracted and passed on.
> 
> Forgive me if I'm being dim (and in particular, please forgive me if I'm
> going over something that was already discussed; I know it's been a
> while). But I don't see why it needs to be passed through the core MTD
> code.
> 
> To take the simple case of an unpartitioned MTD device -- why can't the
> map driver (or whatever) just call the maccessor setup function for
> itself, directly, right after calling add_mtd_device() with its
> newly-probed MTD device?
> 
> And for partitions, why can't it do the same, on the appropriate
> partition.
> 
> OK, the answer to the latter question is that you don't actually *have*
> the pointers to each partition you register. But that's easily fixed.
> 
> If we make add_mtd_partitions() take an extra 'struct mtd_info **'
> argument and put pointers to the slave mtd 'devices' into that, it means
> that your board driver *can* reliably get the mtd pointer for the fourth
> partition, or whatever it needs. And can then just do the memory
> accessor setup for itself.
> 
> Isn't that enough?
> 

Thanks for the feedback. I'll be re-working on this patch and will re-post
the updated patch soon.

Regards,
Sudhakar

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

* RE: [PATCH 1/2] mtdpart: memory accessor interface for MTD layer
  2010-08-06  6:48             ` Sudhakar Rajashekhara
@ 2010-08-08 12:23                 ` David Woodhouse
  0 siblings, 0 replies; 29+ messages in thread
From: David Woodhouse @ 2010-08-08 12:23 UTC (permalink / raw)
  To: Sudhakar Rajashekhara
  Cc: 'David Brownell', 'Bernd Schmidt',
	'Nicolas Pitre', 'Kevin Hilman',
	linux-kernel, 'David Howells', 'David Brownell',
	linux-mtd, 'Andrew Morton'

On Fri, 2010-08-06 at 12:18 +0530, Sudhakar Rajashekhara wrote:
> 
> Thanks for the feedback. I'll be re-working on this patch and will
> re-post
> the updated patch soon. 

Start with this, perhaps...

Subject: mtd/partitions: Add add_mtd_partitions_ret() function

Some callers want access to the MTD devices which get registered for
them when they call add_mtd_partitions(). Add a variant on the function
which does that.

Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>

diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 4c539de..b9ee79b 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -522,26 +522,36 @@ out_register:
  * for reasons of data integrity.
  */
 
-int add_mtd_partitions(struct mtd_info *master,
-		       const struct mtd_partition *parts,
-		       int nbparts)
+int add_mtd_partitions_ret(struct mtd_info *master,
+			   const struct mtd_partition *parts,
+			   int nbparts, struct mtd_info ***mtds_ret)
 {
 	struct mtd_part *slave;
 	uint64_t cur_offset = 0;
+	struct mtd_info **mtds = NULL;
 	int i;
 
 	printk(KERN_NOTICE "Creating %d MTD partitions on \"%s\":\n", nbparts, master->name);
 
+	if (mtds_ret) {
+		mtds = kmalloc(sizeof(*mtds) * nbparts, GFP_KERNEL);
+		if (!mtds)
+			return -ENOMEM;
+	}
 	for (i = 0; i < nbparts; i++) {
 		slave = add_one_partition(master, parts + i, i, cur_offset);
-		if (!slave)
+		if (!slave) {
+			kfree(mtds);
 			return -ENOMEM;
+		}
 		cur_offset = slave->offset + slave->mtd.size;
 	}
 
+	if (mtds_ret)
+		*mtds_ret = mtds;
 	return 0;
 }
-EXPORT_SYMBOL(add_mtd_partitions);
+EXPORT_SYMBOL_GPL(add_mtd_partitions_ret);
 
 static DEFINE_SPINLOCK(part_parser_lock);
 static LIST_HEAD(part_parsers);
diff --git a/include/linux/mtd/partitions.h b/include/linux/mtd/partitions.h
index 274b619..f7935fa 100644
--- a/include/linux/mtd/partitions.h
+++ b/include/linux/mtd/partitions.h
@@ -49,9 +49,12 @@ struct mtd_partition {
 
 struct mtd_info;
 
-int add_mtd_partitions(struct mtd_info *, const struct mtd_partition *, int);
+int add_mtd_partitions_ret(struct mtd_info *, const struct mtd_partition *, int,
+			   struct mtd_info ***);
 int del_mtd_partitions(struct mtd_info *);
 
+#define add_mtd_partitions(m, p, n) add_mtd_partitions_ret(m, p, n, NULL)
+
 /*
  * Functions dealing with the various ways of partitioning the space
  */

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation


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

* RE: [PATCH 1/2] mtdpart: memory accessor interface for MTD layer
@ 2010-08-08 12:23                 ` David Woodhouse
  0 siblings, 0 replies; 29+ messages in thread
From: David Woodhouse @ 2010-08-08 12:23 UTC (permalink / raw)
  To: Sudhakar Rajashekhara
  Cc: 'Bernd Schmidt', 'David Howells',
	'David Brownell', 'Nicolas Pitre',
	'Kevin Hilman', linux-kernel, 'David Brownell',
	linux-mtd, 'Andrew Morton'

On Fri, 2010-08-06 at 12:18 +0530, Sudhakar Rajashekhara wrote:
> 
> Thanks for the feedback. I'll be re-working on this patch and will
> re-post
> the updated patch soon. 

Start with this, perhaps...

Subject: mtd/partitions: Add add_mtd_partitions_ret() function

Some callers want access to the MTD devices which get registered for
them when they call add_mtd_partitions(). Add a variant on the function
which does that.

Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>

diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 4c539de..b9ee79b 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -522,26 +522,36 @@ out_register:
  * for reasons of data integrity.
  */
 
-int add_mtd_partitions(struct mtd_info *master,
-		       const struct mtd_partition *parts,
-		       int nbparts)
+int add_mtd_partitions_ret(struct mtd_info *master,
+			   const struct mtd_partition *parts,
+			   int nbparts, struct mtd_info ***mtds_ret)
 {
 	struct mtd_part *slave;
 	uint64_t cur_offset = 0;
+	struct mtd_info **mtds = NULL;
 	int i;
 
 	printk(KERN_NOTICE "Creating %d MTD partitions on \"%s\":\n", nbparts, master->name);
 
+	if (mtds_ret) {
+		mtds = kmalloc(sizeof(*mtds) * nbparts, GFP_KERNEL);
+		if (!mtds)
+			return -ENOMEM;
+	}
 	for (i = 0; i < nbparts; i++) {
 		slave = add_one_partition(master, parts + i, i, cur_offset);
-		if (!slave)
+		if (!slave) {
+			kfree(mtds);
 			return -ENOMEM;
+		}
 		cur_offset = slave->offset + slave->mtd.size;
 	}
 
+	if (mtds_ret)
+		*mtds_ret = mtds;
 	return 0;
 }
-EXPORT_SYMBOL(add_mtd_partitions);
+EXPORT_SYMBOL_GPL(add_mtd_partitions_ret);
 
 static DEFINE_SPINLOCK(part_parser_lock);
 static LIST_HEAD(part_parsers);
diff --git a/include/linux/mtd/partitions.h b/include/linux/mtd/partitions.h
index 274b619..f7935fa 100644
--- a/include/linux/mtd/partitions.h
+++ b/include/linux/mtd/partitions.h
@@ -49,9 +49,12 @@ struct mtd_partition {
 
 struct mtd_info;
 
-int add_mtd_partitions(struct mtd_info *, const struct mtd_partition *, int);
+int add_mtd_partitions_ret(struct mtd_info *, const struct mtd_partition *, int,
+			   struct mtd_info ***);
 int del_mtd_partitions(struct mtd_info *);
 
+#define add_mtd_partitions(m, p, n) add_mtd_partitions_ret(m, p, n, NULL)
+
 /*
  * Functions dealing with the various ways of partitioning the space
  */

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

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

* RE: [PATCH 1/2] mtdpart: memory accessor interface for MTD layer
  2010-08-08 12:23                 ` David Woodhouse
  (?)
@ 2010-08-09 11:55                 ` Sudhakar Rajashekhara
  -1 siblings, 0 replies; 29+ messages in thread
From: Sudhakar Rajashekhara @ 2010-08-09 11:55 UTC (permalink / raw)
  To: 'David Woodhouse'
  Cc: 'Bernd Schmidt', 'David Brownell',
	'David Brownell', 'Nicolas Pitre',
	'Kevin Hilman', linux-kernel, 'David Howells',
	linux-mtd, 'Andrew Morton'

Hi David,

On Sun, Aug 08, 2010 at 17:53:18, David Woodhouse wrote:
> On Fri, 2010-08-06 at 12:18 +0530, Sudhakar Rajashekhara wrote:
> > 
> > Thanks for the feedback. I'll be re-working on this patch and will
> > re-post
> > the updated patch soon. 
> 
> Start with this, perhaps...
> 
> Subject: mtd/partitions: Add add_mtd_partitions_ret() function
> 
> Some callers want access to the MTD devices which get registered for
> them when they call add_mtd_partitions(). Add a variant on the function
> which does that.
> 
> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
> 
> diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
> index 4c539de..b9ee79b 100644
> --- a/drivers/mtd/mtdpart.c
> +++ b/drivers/mtd/mtdpart.c
> @@ -522,26 +522,36 @@ out_register:
>   * for reasons of data integrity.
>   */
>  
> -int add_mtd_partitions(struct mtd_info *master,
> -		       const struct mtd_partition *parts,
> -		       int nbparts)
> +int add_mtd_partitions_ret(struct mtd_info *master,
> +			   const struct mtd_partition *parts,
> +			   int nbparts, struct mtd_info ***mtds_ret)
>  {
>  	struct mtd_part *slave;
>  	uint64_t cur_offset = 0;
> +	struct mtd_info **mtds = NULL;
>  	int i;
>  
>  	printk(KERN_NOTICE "Creating %d MTD partitions on \"%s\":\n", nbparts, master->name);
>  
> +	if (mtds_ret) {
> +		mtds = kmalloc(sizeof(*mtds) * nbparts, GFP_KERNEL);
> +		if (!mtds)
> +			return -ENOMEM;
> +	}
>  	for (i = 0; i < nbparts; i++) {
>  		slave = add_one_partition(master, parts + i, i, cur_offset);
> -		if (!slave)
> +		if (!slave) {
> +			kfree(mtds);
>  			return -ENOMEM;
> +		}
>  		cur_offset = slave->offset + slave->mtd.size;
>  	}
>  
> +	if (mtds_ret)
> +		*mtds_ret = mtds;
>  	return 0;
>  }
> -EXPORT_SYMBOL(add_mtd_partitions);
> +EXPORT_SYMBOL_GPL(add_mtd_partitions_ret);
>  
>  static DEFINE_SPINLOCK(part_parser_lock);
>  static LIST_HEAD(part_parsers);
> diff --git a/include/linux/mtd/partitions.h b/include/linux/mtd/partitions.h
> index 274b619..f7935fa 100644
> --- a/include/linux/mtd/partitions.h
> +++ b/include/linux/mtd/partitions.h
> @@ -49,9 +49,12 @@ struct mtd_partition {
>  
>  struct mtd_info;
>  
> -int add_mtd_partitions(struct mtd_info *, const struct mtd_partition *, int);
> +int add_mtd_partitions_ret(struct mtd_info *, const struct mtd_partition *, int,
> +			   struct mtd_info ***);
>  int del_mtd_partitions(struct mtd_info *);
>  
> +#define add_mtd_partitions(m, p, n) add_mtd_partitions_ret(m, p, n, NULL)
> +
>  /*
>   * Functions dealing with the various ways of partitioning the space
>   */
> 
> -- 

Thanks very much for this piece of code, it reduced my work. But while working
on this, I found out that, for m25p80 device, the MTD device is available even
for the add_mtd_partitions() case. So the method to read the MAC address remains
same both for un-partitioned and partitioned MTD device. I'll post the modified
patch tomorrow. I'll be looking forward to hearing your comments on that patch
as well.

Thanks,
Sudhakar 

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

* [patch 1/2] mtdpart: memory accessor interface for MTD layer
@ 2010-10-20 22:59 akpm
  0 siblings, 0 replies; 29+ messages in thread
From: akpm @ 2010-10-20 22:59 UTC (permalink / raw)
  To: dwmw2; +Cc: sudhakar.raj, khilman, david-b, linux-mtd, cbouatmailru, akpm

From: Sudhakar Rajashekhara <sudhakar.raj@ti.com>

Implement a memory accessor interface in the MTD layer which enables the
kernel to access flash data.

This patch adds two new members to the mtd_partition structure, a function
handler which will be called during setup of the partition and an argument
to be passed to this setup function.

Example:
+static struct mtd_partition spi_flash_partitions[] = {
+       [0] = {
+               .name       = "U-Boot",
+               .offset     = 0,
+               .size       = SZ_256K,
+               .mask_flags = MTD_WRITEABLE,
+       },
+       [1] = {
+               .name       = "U-Boot Environment",
+               .offset     = MTDPART_OFS_NXTBLK,
+               .size       = SZ_64K,
+               .mask_flags = MTD_WRITEABLE,
+       },
+       [2] = {
+               .name       = "Linux",
+               .offset     = MTDPART_OFS_NXTBLK,
+               .size       = SZ_7M,
+               .mask_flags = 0,
+       },
+       [3] = {
+               .name       = "MAC Address",
+               .offset     = MTDPART_OFS_NXTBLK,
+               .size       = SZ_64K,
+               .mask_flags = 0,
+               .setup      = davinci_get_mac_addr,
+               .context    = (void *)0,
+       },
+};

The davinci_get_mac_addr function reads the MAC address from
offset ZERO of last MTD partition.

Signed-off-by: Sudhakar Rajashekhara <sudhakar.raj@ti.com>
Acked-by: Kevin Hilman <khilman@deeprootsystems.com>
Cc: David Brownell <david-b@pacbell.net>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Anton Vorontsov <cbouatmailru@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 drivers/mtd/mtdpart.c          |   40 +++++++++++++++++++++++++++++++
 include/linux/mtd/partitions.h |    3 ++
 2 files changed, 43 insertions(+)

diff -puN drivers/mtd/mtdpart.c~mtdpart-memory-accessor-interface-for-mtd-layer drivers/mtd/mtdpart.c
--- a/drivers/mtd/mtdpart.c~mtdpart-memory-accessor-interface-for-mtd-layer
+++ a/drivers/mtd/mtdpart.c
@@ -37,6 +37,7 @@ static LIST_HEAD(mtd_partitions);
 struct mtd_part {
 	struct mtd_info mtd;
 	struct mtd_info *master;
+	struct memory_accessor macc;
 	uint64_t offset;
 	struct list_head list;
 };
@@ -346,6 +347,39 @@ int del_mtd_partitions(struct mtd_info *
 }
 EXPORT_SYMBOL(del_mtd_partitions);
 
+/*
+ * This lets other kernel code access the flash data. For example, it
+ * might hold a board's Ethernet address, or board-specific calibration
+ * data generated on the manufacturing floor.
+ */
+static ssize_t mtd_macc_read(struct memory_accessor *macc, char *buf,
+			off_t offset, size_t count)
+{
+	struct mtd_part *part = container_of(macc, struct mtd_part, macc);
+	ssize_t ret = -EIO;
+	size_t retlen;
+
+	if (part_read((struct mtd_info *)part, offset, count,
+			&retlen, buf) == 0)
+		ret = retlen;
+
+	return ret;
+}
+
+static ssize_t mtd_macc_write(struct memory_accessor *macc, const char *buf,
+			off_t offset, size_t count)
+{
+	struct mtd_part *part = container_of(macc, struct mtd_part, macc);
+	ssize_t ret = -EIO;
+	size_t retlen;
+
+	if (part_write((struct mtd_info *)part, offset, count,
+			&retlen, buf) == 0)
+		ret = retlen;
+
+	return ret;
+}
+
 static struct mtd_part *add_one_partition(struct mtd_info *master,
 		const struct mtd_partition *part, int partno,
 		uint64_t cur_offset)
@@ -383,6 +417,9 @@ static struct mtd_part *add_one_partitio
 	slave->mtd.read = part_read;
 	slave->mtd.write = part_write;
 
+	slave->macc.read = mtd_macc_read;
+	slave->macc.write = mtd_macc_write;
+
 	if (master->panic_write)
 		slave->mtd.panic_write = part_panic_write;
 
@@ -449,6 +486,9 @@ static struct mtd_part *add_one_partitio
 	printk(KERN_NOTICE "0x%012llx-0x%012llx : \"%s\"\n", (unsigned long long)slave->offset,
 		(unsigned long long)(slave->offset + slave->mtd.size), slave->mtd.name);
 
+	if (part->setup)
+		part->setup(&slave->macc, (void *)part->context);
+
 	/* let's do some sanity checks */
 	if (slave->offset >= master->size) {
 		/* let's register it anyway to preserve ordering */
diff -puN include/linux/mtd/partitions.h~mtdpart-memory-accessor-interface-for-mtd-layer include/linux/mtd/partitions.h
--- a/include/linux/mtd/partitions.h~mtdpart-memory-accessor-interface-for-mtd-layer
+++ a/include/linux/mtd/partitions.h
@@ -10,6 +10,7 @@
 #define MTD_PARTITIONS_H
 
 #include <linux/types.h>
+#include <linux/memory.h>
 
 
 /*
@@ -40,6 +41,8 @@ struct mtd_partition {
 	uint64_t offset;		/* offset within the master MTD space */
 	uint32_t mask_flags;		/* master MTD flags to mask out for this partition */
 	struct nand_ecclayout *ecclayout;	/* out of band layout for this partition (NAND only)*/
+	void (*setup)(struct memory_accessor *, void *context);
+	void *context;
 };
 
 #define MTDPART_OFS_NXTBLK	(-2)
_

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

* RE: [patch 1/2] mtdpart: memory accessor interface for MTD layer
  2010-10-02 14:44   ` David Woodhouse
@ 2010-10-04 14:28     ` Sudhakar Rajashekhara
  0 siblings, 0 replies; 29+ messages in thread
From: Sudhakar Rajashekhara @ 2010-10-04 14:28 UTC (permalink / raw)
  To: 'David Woodhouse', dedekind1
  Cc: khilman, david-b, akpm, linux-mtd, cbouatmailru

All,

On Sat, Oct 02, 2010 at 20:14:32, David Woodhouse wrote:
> On Sat, 2010-10-02 at 17:09 +0300, Artem Bityutskiy wrote:
> > 
> > > Implement a memory accessor interface in the MTD layer which enables
> > the
> > > kernel to access flash data.
> > > 
> > > This patch adds two new members to the mtd_partition structure, a
> > function
> > > handler which will be called during setup of the partition and an
> > argument
> > > to be passed to this setup function.
> > 
> > This patch does not apply to my l2-mtd-2.6.git tree
> 
> Wasn't there going to be a much nicer version of this patch coming
> anyway?
> 

This patch can be dropped. Please see the discussion at [1] regarding this.

[1] http://lists.infradead.org/pipermail/linux-mtd/2010-August/031483.html

Thanks,
Sudhakar

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

* Re: [patch 1/2] mtdpart: memory accessor interface for MTD layer
  2010-10-02 14:09 ` Artem Bityutskiy
  2010-10-02 14:44   ` David Woodhouse
@ 2010-10-02 19:09   ` Andrew Morton
  1 sibling, 0 replies; 29+ messages in thread
From: Andrew Morton @ 2010-10-02 19:09 UTC (permalink / raw)
  To: dedekind1; +Cc: sudhakar.raj, khilman, david-b, linux-mtd, cbouatmailru, dwmw2

On Sat, 02 Oct 2010 17:09:52 +0300 Artem Bityutskiy <dedekind1@gmail.com> wrote:

> On Fri, 2010-10-01 at 14:16 -0700, akpm@linux-foundation.org wrote:
> > From: Sudhakar Rajashekhara <sudhakar.raj@ti.com>
> > 
> > Implement a memory accessor interface in the MTD layer which enables the
> > kernel to access flash data.
> > 
> > This patch adds two new members to the mtd_partition structure, a function
> > handler which will be called during setup of the partition and an argument
> > to be passed to this setup function.
> 
> This patch does not apply to my l2-mtd-2.6.git tree - many confilicts.

It applies against linux-next.

For heavens sake guys, please fix this.

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

* Re: [patch 1/2] mtdpart: memory accessor interface for MTD layer
  2010-10-02 14:09 ` Artem Bityutskiy
@ 2010-10-02 14:44   ` David Woodhouse
  2010-10-04 14:28     ` Sudhakar Rajashekhara
  2010-10-02 19:09   ` Andrew Morton
  1 sibling, 1 reply; 29+ messages in thread
From: David Woodhouse @ 2010-10-02 14:44 UTC (permalink / raw)
  To: dedekind1; +Cc: sudhakar.raj, khilman, david-b, linux-mtd, cbouatmailru, akpm

On Sat, 2010-10-02 at 17:09 +0300, Artem Bityutskiy wrote:
> 
> > Implement a memory accessor interface in the MTD layer which enables
> the
> > kernel to access flash data.
> > 
> > This patch adds two new members to the mtd_partition structure, a
> function
> > handler which will be called during setup of the partition and an
> argument
> > to be passed to this setup function.
> 
> This patch does not apply to my l2-mtd-2.6.git tree

Wasn't there going to be a much nicer version of this patch coming
anyway?

-- 
dwmw2

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

* Re: [patch 1/2] mtdpart: memory accessor interface for MTD layer
  2010-10-01 21:16 [patch " akpm
@ 2010-10-02 14:09 ` Artem Bityutskiy
  2010-10-02 14:44   ` David Woodhouse
  2010-10-02 19:09   ` Andrew Morton
  0 siblings, 2 replies; 29+ messages in thread
From: Artem Bityutskiy @ 2010-10-02 14:09 UTC (permalink / raw)
  To: akpm; +Cc: sudhakar.raj, khilman, david-b, linux-mtd, cbouatmailru, dwmw2

On Fri, 2010-10-01 at 14:16 -0700, akpm@linux-foundation.org wrote:
> From: Sudhakar Rajashekhara <sudhakar.raj@ti.com>
> 
> Implement a memory accessor interface in the MTD layer which enables the
> kernel to access flash data.
> 
> This patch adds two new members to the mtd_partition structure, a function
> handler which will be called during setup of the partition and an argument
> to be passed to this setup function.

This patch does not apply to my l2-mtd-2.6.git tree - many confilicts.
Sudhakar, you can refresh your patch against my tree and re-send:

http://git.infradead.org/users/dedekind/l2-mtd-2.6.git

-- 
Best Regards,
Artem Bityutskiy (Битюцкий Артём)

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

* [patch 1/2] mtdpart: memory accessor interface for MTD layer
@ 2010-10-01 21:16 akpm
  2010-10-02 14:09 ` Artem Bityutskiy
  0 siblings, 1 reply; 29+ messages in thread
From: akpm @ 2010-10-01 21:16 UTC (permalink / raw)
  To: dwmw2; +Cc: sudhakar.raj, khilman, david-b, linux-mtd, cbouatmailru, akpm

From: Sudhakar Rajashekhara <sudhakar.raj@ti.com>

Implement a memory accessor interface in the MTD layer which enables the
kernel to access flash data.

This patch adds two new members to the mtd_partition structure, a function
handler which will be called during setup of the partition and an argument
to be passed to this setup function.

Example:
+static struct mtd_partition spi_flash_partitions[] = {
+       [0] = {
+               .name       = "U-Boot",
+               .offset     = 0,
+               .size       = SZ_256K,
+               .mask_flags = MTD_WRITEABLE,
+       },
+       [1] = {
+               .name       = "U-Boot Environment",
+               .offset     = MTDPART_OFS_NXTBLK,
+               .size       = SZ_64K,
+               .mask_flags = MTD_WRITEABLE,
+       },
+       [2] = {
+               .name       = "Linux",
+               .offset     = MTDPART_OFS_NXTBLK,
+               .size       = SZ_7M,
+               .mask_flags = 0,
+       },
+       [3] = {
+               .name       = "MAC Address",
+               .offset     = MTDPART_OFS_NXTBLK,
+               .size       = SZ_64K,
+               .mask_flags = 0,
+               .setup      = davinci_get_mac_addr,
+               .context    = (void *)0,
+       },
+};

The davinci_get_mac_addr function reads the MAC address from
offset ZERO of last MTD partition.

Signed-off-by: Sudhakar Rajashekhara <sudhakar.raj@ti.com>
Acked-by: Kevin Hilman <khilman@deeprootsystems.com>
Cc: David Brownell <david-b@pacbell.net>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Anton Vorontsov <cbouatmailru@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 drivers/mtd/mtdpart.c          |   40 +++++++++++++++++++++++++++++++
 include/linux/mtd/partitions.h |    3 ++
 2 files changed, 43 insertions(+)

diff -puN drivers/mtd/mtdpart.c~mtdpart-memory-accessor-interface-for-mtd-layer drivers/mtd/mtdpart.c
--- a/drivers/mtd/mtdpart.c~mtdpart-memory-accessor-interface-for-mtd-layer
+++ a/drivers/mtd/mtdpart.c
@@ -37,6 +37,7 @@ static LIST_HEAD(mtd_partitions);
 struct mtd_part {
 	struct mtd_info mtd;
 	struct mtd_info *master;
+	struct memory_accessor macc;
 	uint64_t offset;
 	struct list_head list;
 };
@@ -346,6 +347,39 @@ int del_mtd_partitions(struct mtd_info *
 }
 EXPORT_SYMBOL(del_mtd_partitions);
 
+/*
+ * This lets other kernel code access the flash data. For example, it
+ * might hold a board's Ethernet address, or board-specific calibration
+ * data generated on the manufacturing floor.
+ */
+static ssize_t mtd_macc_read(struct memory_accessor *macc, char *buf,
+			off_t offset, size_t count)
+{
+	struct mtd_part *part = container_of(macc, struct mtd_part, macc);
+	ssize_t ret = -EIO;
+	size_t retlen;
+
+	if (part_read((struct mtd_info *)part, offset, count,
+			&retlen, buf) == 0)
+		ret = retlen;
+
+	return ret;
+}
+
+static ssize_t mtd_macc_write(struct memory_accessor *macc, const char *buf,
+			off_t offset, size_t count)
+{
+	struct mtd_part *part = container_of(macc, struct mtd_part, macc);
+	ssize_t ret = -EIO;
+	size_t retlen;
+
+	if (part_write((struct mtd_info *)part, offset, count,
+			&retlen, buf) == 0)
+		ret = retlen;
+
+	return ret;
+}
+
 static struct mtd_part *add_one_partition(struct mtd_info *master,
 		const struct mtd_partition *part, int partno,
 		uint64_t cur_offset)
@@ -383,6 +417,9 @@ static struct mtd_part *add_one_partitio
 	slave->mtd.read = part_read;
 	slave->mtd.write = part_write;
 
+	slave->macc.read = mtd_macc_read;
+	slave->macc.write = mtd_macc_write;
+
 	if (master->panic_write)
 		slave->mtd.panic_write = part_panic_write;
 
@@ -449,6 +486,9 @@ static struct mtd_part *add_one_partitio
 	printk(KERN_NOTICE "0x%012llx-0x%012llx : \"%s\"\n", (unsigned long long)slave->offset,
 		(unsigned long long)(slave->offset + slave->mtd.size), slave->mtd.name);
 
+	if (part->setup)
+		part->setup(&slave->macc, (void *)part->context);
+
 	/* let's do some sanity checks */
 	if (slave->offset >= master->size) {
 		/* let's register it anyway to preserve ordering */
diff -puN include/linux/mtd/partitions.h~mtdpart-memory-accessor-interface-for-mtd-layer include/linux/mtd/partitions.h
--- a/include/linux/mtd/partitions.h~mtdpart-memory-accessor-interface-for-mtd-layer
+++ a/include/linux/mtd/partitions.h
@@ -10,6 +10,7 @@
 #define MTD_PARTITIONS_H
 
 #include <linux/types.h>
+#include <linux/memory.h>
 
 
 /*
@@ -40,6 +41,8 @@ struct mtd_partition {
 	uint64_t offset;		/* offset within the master MTD space */
 	uint32_t mask_flags;		/* master MTD flags to mask out for this partition */
 	struct nand_ecclayout *ecclayout;	/* out of band layout for this partition (NAND only)*/
+	void (*setup)(struct memory_accessor *, void *context);
+	void *context;
 };
 
 #define MTDPART_OFS_NXTBLK	(-2)
_

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

end of thread, other threads:[~2010-10-20 22:59 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-03-16 20:41 [PATCH 1/2] mtdpart: memory accessor interface for MTD layer Kevin Hilman
2010-03-16 20:41 ` Kevin Hilman
2010-04-08  8:10 ` Artem Bityutskiy
2010-04-08  8:10   ` Artem Bityutskiy
2010-05-13 23:50 ` David Woodhouse
2010-05-13 23:50   ` David Woodhouse
2010-07-07 10:56   ` Sudhakar Rajashekhara
2010-07-07 11:08     ` David Brownell
2010-07-07 11:08       ` David Brownell
2010-07-08 15:10       ` Sudhakar Rajashekhara
2010-07-08 16:00         ` David Brownell
2010-08-04 10:12       ` David Woodhouse
2010-08-04 10:12         ` David Woodhouse
2010-08-04 10:31         ` David Brownell
2010-08-04 10:31           ` David Brownell
2010-08-04 11:08           ` David Woodhouse
2010-08-04 11:08             ` David Woodhouse
2010-08-04 11:27             ` David Brownell
2010-08-04 11:27               ` David Brownell
2010-08-06  6:48             ` Sudhakar Rajashekhara
2010-08-08 12:23               ` David Woodhouse
2010-08-08 12:23                 ` David Woodhouse
2010-08-09 11:55                 ` Sudhakar Rajashekhara
2010-10-01 21:16 [patch " akpm
2010-10-02 14:09 ` Artem Bityutskiy
2010-10-02 14:44   ` David Woodhouse
2010-10-04 14:28     ` Sudhakar Rajashekhara
2010-10-02 19:09   ` Andrew Morton
2010-10-20 22:59 akpm

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