diff mbox

[OpenWrt-Devel,RFC,bcm53xx] mtd: nand: bcm_nand: switch from cmdlinepart to ofpart

Message ID 1417911502-20517-1-git-send-email-zajec5@gmail.com
State RFC
Headers show

Commit Message

Rafał Miłecki Dec. 7, 2014, 12:18 a.m. UTC
bcm_nand uses (and depends on) OF, so there isn't much sense to use
cmdlinepart

Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
---
 drivers/mtd/nand/bcm_nand.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

Hauke Mehrtens Dec. 7, 2014, 3:08 p.m. UTC | #1
On 12/07/2014 01:18 AM, Rafał Miłecki wrote:
> bcm_nand uses (and depends on) OF, so there isn't much sense to use
> cmdlinepart
> 
> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
> ---
>  drivers/mtd/nand/bcm_nand.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/nand/bcm_nand.c b/drivers/mtd/nand/bcm_nand.c
> index 3c5b1d1..f2bd79e 100644
> --- a/drivers/mtd/nand/bcm_nand.c
> +++ b/drivers/mtd/nand/bcm_nand.c
> @@ -40,6 +40,7 @@
>  
>  #include <linux/mtd/mtd.h>
>  #include <linux/mtd/nand.h>
> +#include <linux/mtd/partitions.h>
>  
>  #define NANDC_MAX_CHIPS		2	/* Only 2 CSn supported in NorthStar */
>  
> @@ -1494,13 +1495,14 @@ static int __init bcmnand_idm_init(struct bcmnand_ctrl *ctrl)
>  	return 0;
>  }
>  
> -static const char * const part_probes[] = { "bcm47xxpart", "cmdlinepart", NULL };
> +static const char * const part_probes[] = { "bcm47xxpart", "ofpart", NULL };

This makes sense. Shouldn't ofpart be the first thing we probe for? This
way we can provide a partition map through device tree and overwrite the
auto probed one.

>  
>  /*
>   * Top-level init function
>   */
>  static int bcmnand_probe(struct bcma_device *core)
>  {
> +	struct mtd_part_parser_data parser_data;
>  	struct device *dev = &core->dev;
>  	struct device_node *np = dev->of_node;
>  	struct bcmnand_ctrl *ctrl;
> @@ -1539,7 +1541,8 @@ static int bcmnand_probe(struct bcma_device *core)
>  	if (res)
>  		return res;
>  
> -	res = mtd_device_parse_register(&ctrl->mtd, part_probes, NULL, NULL, 0);
> +	parser_data.of_node = np;
> +	res = mtd_device_parse_register(&ctrl->mtd, part_probes, &parser_data, NULL, 0);

Is this needed for ofpart?

>  	if (res) {
>  		pr_err("%s: Failed to register MTD device: %d\n", DRV_NAME, res);
>  		return res;
>
Rafał Miłecki Dec. 7, 2014, 3:12 p.m. UTC | #2
On 7 December 2014 at 16:08, Hauke Mehrtens <hauke@hauke-m.de> wrote:
> On 12/07/2014 01:18 AM, Rafał Miłecki wrote:
>> bcm_nand uses (and depends on) OF, so there isn't much sense to use
>> cmdlinepart
>>
>> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
>> ---
>>  drivers/mtd/nand/bcm_nand.c | 7 +++++--
>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/bcm_nand.c b/drivers/mtd/nand/bcm_nand.c
>> index 3c5b1d1..f2bd79e 100644
>> --- a/drivers/mtd/nand/bcm_nand.c
>> +++ b/drivers/mtd/nand/bcm_nand.c
>> @@ -40,6 +40,7 @@
>>
>>  #include <linux/mtd/mtd.h>
>>  #include <linux/mtd/nand.h>
>> +#include <linux/mtd/partitions.h>
>>
>>  #define NANDC_MAX_CHIPS              2       /* Only 2 CSn supported in NorthStar */
>>
>> @@ -1494,13 +1495,14 @@ static int __init bcmnand_idm_init(struct bcmnand_ctrl *ctrl)
>>       return 0;
>>  }
>>
>> -static const char * const part_probes[] = { "bcm47xxpart", "cmdlinepart", NULL };
>> +static const char * const part_probes[] = { "bcm47xxpart", "ofpart", NULL };
>
> This makes sense. Shouldn't ofpart be the first thing we probe for? This
> way we can provide a partition map through device tree and overwrite the
> auto probed one.

You're right, I'll change the order.


>>  /*
>>   * Top-level init function
>>   */
>>  static int bcmnand_probe(struct bcma_device *core)
>>  {
>> +     struct mtd_part_parser_data parser_data;
>>       struct device *dev = &core->dev;
>>       struct device_node *np = dev->of_node;
>>       struct bcmnand_ctrl *ctrl;
>> @@ -1539,7 +1541,8 @@ static int bcmnand_probe(struct bcma_device *core)
>>       if (res)
>>               return res;
>>
>> -     res = mtd_device_parse_register(&ctrl->mtd, part_probes, NULL, NULL, 0);
>> +     parser_data.of_node = np;
>> +     res = mtd_device_parse_register(&ctrl->mtd, part_probes, &parser_data, NULL, 0);
>
> Is this needed for ofpart?

Yes, see drivers/mtd/ofpart.c. There is
struct mtd_part_parser_data *data
and the code calls
node = data->of_node;
diff mbox

Patch

diff --git a/drivers/mtd/nand/bcm_nand.c b/drivers/mtd/nand/bcm_nand.c
index 3c5b1d1..f2bd79e 100644
--- a/drivers/mtd/nand/bcm_nand.c
+++ b/drivers/mtd/nand/bcm_nand.c
@@ -40,6 +40,7 @@ 
 
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/nand.h>
+#include <linux/mtd/partitions.h>
 
 #define NANDC_MAX_CHIPS		2	/* Only 2 CSn supported in NorthStar */
 
@@ -1494,13 +1495,14 @@  static int __init bcmnand_idm_init(struct bcmnand_ctrl *ctrl)
 	return 0;
 }
 
-static const char * const part_probes[] = { "bcm47xxpart", "cmdlinepart", NULL };
+static const char * const part_probes[] = { "bcm47xxpart", "ofpart", NULL };
 
 /*
  * Top-level init function
  */
 static int bcmnand_probe(struct bcma_device *core)
 {
+	struct mtd_part_parser_data parser_data;
 	struct device *dev = &core->dev;
 	struct device_node *np = dev->of_node;
 	struct bcmnand_ctrl *ctrl;
@@ -1539,7 +1541,8 @@  static int bcmnand_probe(struct bcma_device *core)
 	if (res)
 		return res;
 
-	res = mtd_device_parse_register(&ctrl->mtd, part_probes, NULL, NULL, 0);
+	parser_data.of_node = np;
+	res = mtd_device_parse_register(&ctrl->mtd, part_probes, &parser_data, NULL, 0);
 	if (res) {
 		pr_err("%s: Failed to register MTD device: %d\n", DRV_NAME, res);
 		return res;