Skip to contentSkip to conversation

Generic Platform NAND Driver

PATCH Semih Hazar 2 participants 6 messages May 18, 2007 – May 22, 2007

Subsystems (2)
Memory technology devices (MTD) NAND flash
lore
Share & tools
lore

Semih Hazar

author
#0 lore raw

Hi,

In the new generic platforn NAND driver, line 54;

	data->chip.priv = &data;
	data->mtd.priv = &data->chip;

mtd->priv->priv points to struct plat_nand_data which is defined in the plat_nand.c and never used by it. Can we change it so that this priv points to a host specific data, and the helper functions (board specific) can make use of it. Below is a suggested patch:

include/linux/mtd/nand.h +1 -0
index d2365c8..256957b 100644
@@ -599,10 +599,12 @@ struct platform_nand_ctrl {
 * struct platform_nand_data - container structure for platform-specific data
 * @chip:		chip level chip structure
 * @ctrl:		controller level device structure
+ * @host:		pointer to host specific data structure
 */

struct platform_nand_data {
	struct platform_nand_chip	chip;
	struct platform_nand_ctrl	ctrl;

+ void *host;
};

/* Some helpers to access the data structures */

drivers/mtd/nand/plat_nand.c +0 -0
index cd725fc..1e47ad3 100644
@@ -51,7 +51,7 @@ static int __init plat_nand_probe(struct platform_device *pdev)

		return -EIO;
	}

- data->chip.priv = &data;

+	data->chip.priv = pdata->host;
	data->mtd.priv = &data->chip;

	data->mtd.owner = THIS_MODULE;
author
#1 lore raw

Hi Vitaly,

I couldn't get any replies from the list and I wonder if I'm missing a point here.

struct plat_nand_data seems useless to users of this generic nand driver, and there is no way of passing user data in mtd structures.

Does that make sense, or am I missing something ?

Also the priv members in platform_nand_chip and platform_nand_ctrl are not used in any way. Maybe a better solution would be to utilize these ?

Quoted 42 lines

Hi,

In the new generic platforn NAND driver, line 54;

	data->chip.priv = &data;
	data->mtd.priv = &data->chip;

mtd->priv->priv points to struct plat_nand_data which is defined in the plat_nand.c and never used by it. Can we change it so that this priv points to a host specific data, and the helper functions (board specific) can make use of it. Below is a suggested patch:

include/linux/mtd/nand.h +2 -0
index d2365c8..256957b 100644
@@ -599,10 +599,12 @@ struct platform_nand_ctrl {
  * struct platform_nand_data - container structure for platform-specific data
  * @chip:		chip level chip structure
  * @ctrl:		controller level device structure
+ * @host:		pointer to host specific data structure
  */
 struct platform_nand_data {
 	struct platform_nand_chip	chip;
 	struct platform_nand_ctrl	ctrl;
+	void				*host;
 };
 
 /* Some helpers to access the data structures */
drivers/mtd/nand/plat_nand.c +1 -1
index cd725fc..1e47ad3 100644
@@ -51,7 +51,7 @@ static int __init plat_nand_probe(struct platform_device *pdev)
 		return -EIO;
 	}
 
-	data->chip.priv = &data;
+	data->chip.priv = pdata->host;
 	data->mtd.priv = &data->chip;
 
 	data->mtd.owner = THIS_MODULE;
#2 lore raw
Quoted 10 lines · Semih Hazar

Hi Vitaly,

I couldn't get any replies from the list and I wonder if I'm missing a point here.

struct plat_nand_data seems useless to users of this generic nand driver, and there is no way of passing user data in mtd structures.

Does that make sense, or am I missing something ?

Also the priv members in platform_nand_chip and platform_nand_ctrl are not used in any way. Maybe a better solution would be to utilize these ?

That structures are for arch related code for NAND flashes that registers are memory mapped but has special h/w control.

You can look at the first (I guess) usage example here:
http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=4385/2

Quoted 50 lines
Quoted 36 lines

Hi,

In the new generic platforn NAND driver, line 54;

	data->chip.priv = &data;
	data->mtd.priv = &data->chip;

mtd->priv->priv points to struct plat_nand_data which is defined in the plat_nand.c and never used by it. Can we change it so that this priv points to a host specific data, and the helper functions (board specific) can make use of it. Below is a suggested patch:

include/linux/mtd/nand.h +2 -0
index d2365c8..256957b 100644
@@ -599,10 +599,12 @@ struct platform_nand_ctrl {
  * struct platform_nand_data - container structure for platform-specific data
  * @chip:		chip level chip structure
  * @ctrl:		controller level device structure
+ * @host:		pointer to host specific data structure
  */
 struct platform_nand_data {
 	struct platform_nand_chip	chip;
 	struct platform_nand_ctrl	ctrl;
+	void				*host;
 };
 
 /* Some helpers to access the data structures */
drivers/mtd/nand/plat_nand.c +0 -0
index cd725fc..1e47ad3 100644

drivers/mtd/nand/plat_nand.c static int __init plat_nand_probe(struct platform_device *pdev) · line 57

Quoted 8 lines
drivers/mtd/nand/plat_nand.c +1 -1
@@ -51,7 +51,7 @@ static int __init plat_nand_probe(struct platform_device *pdev)
 		return -EIO;
 	}
 
-	data->chip.priv = &data;
+	data->chip.priv = pdata->host;
 	data->mtd.priv = &data->chip;
 
 	data->mtd.owner = THIS_MODULE;

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

author
#3 lore raw
Quoted 4 lines · Vladimir A. Barinov

You can look at the first (I guess) usage example here:
http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=4385/2

+ixdp425_flash_nand_cmd_ctrl(struct mtd_info *mtd, int cmd, unsigned int ctrl) +{
+ struct nand_chip *this = mtd->priv;
+ int offset = (int)this->priv;

Here offset is read from nand_chip->priv, which actually points to struct plat_nand_data of plat_nand.c

Am I wrong ?

#4 lore raw

Hi Semih,

Quoted 12 lines · Semih Hazar
Quoted 4 lines · Vladimir A. Barinov

You can look at the first (I guess) usage example here:
http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=4385/2

+ixdp425_flash_nand_cmd_ctrl(struct mtd_info *mtd, int cmd, unsigned int ctrl)

(quoted diff) +3 -0
+{
+    struct nand_chip *this = mtd->priv;
+    int offset = (int)this->priv;
Quoted 3 lines

Here offset is read from nand_chip->priv, which actually points to struct plat_nand_data of plat_nand.c

Am I wrong ?

I agree with you.
And you are right that all things work fine because the mtd_info->priv->priv that points to plat_nand_data is not used.

I agree that the line 54 in plat_nand.c should be removed:

- data->chip.priv = &data;

But does it really usefull to create void * host pointer if it could be done in arch code like the above patch does?

Vladimir

author
#5 lore raw
Quoted 13 lines · Vladimir A. Barinov
Quoted 10 lines

+ixdp425_flash_nand_cmd_ctrl(struct mtd_info *mtd, int cmd, unsigned int ctrl)

(quoted diff) +3 -0
+{
+    struct nand_chip *this = mtd->priv;
+    int offset = (int)this->priv;

Here offset is read from nand_chip->priv, which actually points to struct plat_nand_data of plat_nand.c

Am I wrong ?

I agree with you.
And you are right that all things work fine because the mtd_info->priv->priv that points to plat_nand_data is not used.

Quoted 6 lines

I agree that the line 54 in plat_nand.c should be removed:

- data->chip.priv = &data;

But does it really usefull to create void * host pointer if it could be done in arch code like the above patch does?

I thought board-specific code can make use of this area (just as in your patch) so it's good to be able pass some _known_ data there.

My code which uses generic nand and is very much like the patch you've sent. But we may have board specific CLE, ACE, CE pins so they're not #define'd in the machine code, rather they're passed in a struct from the board code. And I thought the right thing to so that would be to pass that data to the priv member rather than accessing it in a static way.

If it's better to access have these board-specific in a static manner, then I'll just go ahead and change my code that way.

Thanks for your feedback.

Regards,

Signature · 1 line
Semih Hazar