| Message ID | 251532766.70336.1408150059026.JavaMail.zimbra@xes-inc.com |
|---|---|
| State | Rejected |
| Headers | show |
On Fri, 2014-08-15 at 19:47 -0500, Aaron Sierra wrote: > Previously, the OF node defining the IFC NAND controller was being > passed to mtd_device_parse_register(), not the node defining the > partitions. This resulted in no OF-defined partitions being created. This driver probes on "fsl,ifc-nand", not "fsl,ifc". So how is it getting the controller node? What does the device tree look like in which you're seeing this happen? -Scott
----- Original Message ----- > From: "Scott Wood" <scottwood@freescale.com> > Sent: Wednesday, August 20, 2014 6:26:25 PM > > On Fri, 2014-08-15 at 19:47 -0500, Aaron Sierra wrote: > > Previously, the OF node defining the IFC NAND controller was being > > passed to mtd_device_parse_register(), not the node defining the > > partitions. This resulted in no OF-defined partitions being created. > > This driver probes on "fsl,ifc-nand", not "fsl,ifc". So how is it > getting the controller node? > > What does the device tree look like in which you're seeing this happen? > > -Scott > This is the node that is defined in my T1042 device tree: nand0@4,0 { #address-cells = <0>; #size-cells = <0>; compatible = "fsl,ifc-nand"; reg = <4 0x0 0x040000>; nand@0 { #address-cells = <1>; #size-cells = <2>; compatible = "micron,mt29f32g08"; partition@0 { label = "NAND Filesystem"; reg = <0 0x1 0x00000000>; }; }; }; It is based on a node used previously with the fsl_upm NAND driver on a P5020: upm@4,0 { #address-cells = <0>; #size-cells = <0>; compatible = "fsl,upm-nand"; reg = <4 0x0 0x080000>; fsl,upm-addr-offset = <0x10>; fsl,upm-cmd-offset = <0x08>; fsl,upm-wait-flags = <0x1>; chip-delay = <50>; nand@0 { #address-cells = <1>; #size-cells = <2>; compatible = "micron,mt29f32g08"; partition@0 { label = "NAND Filesystem"; reg = <0 0x1 0x00000000>; }; }; }; The patch that I submitted is based on code from the fsl_upm driver: flash_np = of_get_next_child(upm_np, NULL); if (!flash_np) return -ENODEV; [snip] ppdata.of_node = flash_np; ret = mtd_device_parse_register(&fun->mtd, NULL, &ppdata, NULL, 0); err: of_node_put(flash_np); -Aaron
On Wed, 2014-08-20 at 18:45 -0500, Aaron Sierra wrote: > ----- Original Message ----- > > From: "Scott Wood" <scottwood@freescale.com> > > Sent: Wednesday, August 20, 2014 6:26:25 PM > > > > On Fri, 2014-08-15 at 19:47 -0500, Aaron Sierra wrote: > > > Previously, the OF node defining the IFC NAND controller was being > > > passed to mtd_device_parse_register(), not the node defining the > > > partitions. This resulted in no OF-defined partitions being created. > > > > This driver probes on "fsl,ifc-nand", not "fsl,ifc". So how is it > > getting the controller node? > > > > What does the device tree look like in which you're seeing this happen? > > > > -Scott > > > > This is the node that is defined in my T1042 device tree: > > nand0@4,0 { > #address-cells = <0>; > #size-cells = <0>; > compatible = "fsl,ifc-nand"; > reg = <4 0x0 0x040000>; > > nand@0 { > #address-cells = <1>; > #size-cells = <2>; > compatible = "micron,mt29f32g08"; > > partition@0 { > label = "NAND Filesystem"; > reg = <0 0x1 0x00000000>; > }; > }; > }; This is wrong. You shouldn't have a separate nand@0 node. Your patch will break partition detection on all of the existing IFC device trees. > It is based on a node used previously with the fsl_upm NAND driver on > a P5020: Why would you base it on a upm node rather than on an ifc node, or on the IFC binding document (Documentation/devicetree/bindings/memory-controllers/fsl/ifc.txt)? -Scott
----- Original Message ----- > From: "Scott Wood" <scottwood@freescale.com> > Sent: Wednesday, August 20, 2014 6:59:56 PM > > On Wed, 2014-08-20 at 18:45 -0500, Aaron Sierra wrote: > > ----- Original Message ----- > > > From: "Scott Wood" <scottwood@freescale.com> > > > Sent: Wednesday, August 20, 2014 6:26:25 PM > > > > > > On Fri, 2014-08-15 at 19:47 -0500, Aaron Sierra wrote: > > > > Previously, the OF node defining the IFC NAND controller was being > > > > passed to mtd_device_parse_register(), not the node defining the > > > > partitions. This resulted in no OF-defined partitions being created. > > > > > > This driver probes on "fsl,ifc-nand", not "fsl,ifc". So how is it > > > getting the controller node? > > > > > > What does the device tree look like in which you're seeing this happen? > > > > > > -Scott > > > > > > > This is the node that is defined in my T1042 device tree: > > > > nand0@4,0 { > > #address-cells = <0>; > > #size-cells = <0>; > > compatible = "fsl,ifc-nand"; > > reg = <4 0x0 0x040000>; > > > > nand@0 { > > #address-cells = <1>; > > #size-cells = <2>; > > compatible = "micron,mt29f32g08"; > > > > partition@0 { > > label = "NAND Filesystem"; > > reg = <0 0x1 0x00000000>; > > }; > > }; > > }; > > This is wrong. You shouldn't have a separate nand@0 node. > > Your patch will break partition detection on all of the existing IFC > device trees. > > > It is based on a node used previously with the fsl_upm NAND driver on > > a P5020: > > Why would you base it on a upm node rather than on an ifc node, or on > the IFC binding document > (Documentation/devicetree/bindings/memory-controllers/fsl/ifc.txt)? > > -Scott > Ah, I was under the wrong impression that there was a common MTD partitioning format. I didn't realize it was defined by each driver. Thanks for clearing that up. -Aaron
On Wed, 2014-08-20 at 19:15 -0500, Aaron Sierra wrote: > Ah, I was under the wrong impression that there was a common MTD > partitioning format. I didn't realize it was defined by each > driver. Thanks for clearing that up. The partition nodes should take the same form regardless of controller type, but not the node that contains the partition nodes. -Scott
diff --git a/drivers/mtd/nand/fsl_ifc_nand.c b/drivers/mtd/nand/fsl_ifc_nand.c index 7861909..9aeb146 100644 --- a/drivers/mtd/nand/fsl_ifc_nand.c +++ b/drivers/mtd/nand/fsl_ifc_nand.c @@ -1029,7 +1029,6 @@ static int fsl_ifc_nand_probe(struct platform_device *dev) struct device_node *node = dev->dev.of_node; struct mtd_part_parser_data ppdata; - ppdata.of_node = dev->dev.of_node; if (!fsl_ifc_ctrl_dev || !fsl_ifc_ctrl_dev->regs) return -ENODEV; ifc = fsl_ifc_ctrl_dev->regs; @@ -1126,9 +1125,13 @@ static int fsl_ifc_nand_probe(struct platform_device *dev) /* First look for RedBoot table or partitions on the command * line, these take precedence over device tree information */ + ppdata.of_node = of_get_next_child(dev->dev.of_node, NULL); mtd_device_parse_register(&priv->mtd, part_probe_types, &ppdata, NULL, 0); + if (ppdata.of_node) + of_node_put(ppdata.of_node); + dev_info(priv->dev, "IFC NAND device at 0x%llx, bank %d\n", (unsigned long long)res.start, priv->bank); return 0;
Previously, the OF node defining the IFC NAND controller was being passed to mtd_device_parse_register(), not the node defining the partitions. This resulted in no OF-defined partitions being created. Signed-off-by: Aaron Sierra <asierra@xes-inc.com> --- drivers/mtd/nand/fsl_ifc_nand.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)