On Wed, 19 Sep 2007 14:16:46 +1000
David Gibson <[EMAIL PROTECTED]> wrote:
> This patch includes a whole batch of smallish cleanups for
> drivers/mtd/physmap_of.c.
>
> - A bunch of uneeded #includes are removed
> - We switch to the modern linux/of.h etc. in place of
> asm/prom.h
> - Use some helper macros to avoid some ugly inline #ifdefs
> - A few lines of unreachable code are removed
> - A number of indentation / line-wrapping fixes
> - More consistent use of kernel idioms such as if (!p) instead
> of if (p == NULL)
> - Clarify some printk()s and other informative strings.
Most of that looks good. Just a couple issues below. Mostly it
doesn't apply cleanly to my tree because you didn't base if off of the
patch I sent out last week to fix optional partitions.
> - (the big one) Despite the name, this driver really has
> nothing to do with drivers/mtd/physmap.c. The fact that the flash
> chips must be physically direct mapped is a constrant, but doesn't
> really say anything about the actual purpose of this driver, which is
> to instantiate MTD devices based on information from the device tree.
> Therefore the physmap name is replaced everywhere within the file with
> "of_flash". The file itself and the Kconfig option is not renamed for
> now (so that the diff is actually a diff). That can come later.
Later when? If we're all in agreement, then why don't we just apply
your patch after you fix it up and I can move the file in my git tree.
That way we don't forget about it.
> Signed-off-by: David Gibson <[EMAIL PROTECTED]>
>
> Index: working-2.6/drivers/mtd/maps/physmap_of.c
> ===================================================================
> --- working-2.6.orig/drivers/mtd/maps/physmap_of.c 2007-09-14
> 14:24:06.000000000 +1000
> +++ working-2.6/drivers/mtd/maps/physmap_of.c 2007-09-19 13:59:23.000000000
> +1000
> @@ -1,5 +1,5 @@
> /*
> - * Normal mappings of chips in physical memory for OF devices
> + * Flash mappings described by the OF (or flattened) device tree
> *
> * Copyright (C) 2006 MontaVista Software Inc.
> * Author: Vitaly Wool <[EMAIL PROTECTED]>
> @@ -15,20 +15,15 @@
>
> #include <linux/module.h>
> #include <linux/types.h>
> -#include <linux/kernel.h>
> #include <linux/init.h>
> -#include <linux/slab.h>
> #include <linux/device.h>
> #include <linux/mtd/mtd.h>
> #include <linux/mtd/map.h>
> #include <linux/mtd/partitions.h>
> -#include <linux/mtd/physmap.h>
> -#include <asm/io.h>
> -#include <asm/prom.h>
> -#include <asm/of_device.h>
> -#include <asm/of_platform.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
>
> -struct physmap_flash_info {
> +struct of_flash {
> struct mtd_info *mtd;
> struct map_info map;
> struct resource *res;
> @@ -38,8 +33,10 @@ struct physmap_flash_info {
> };
>
> #ifdef CONFIG_MTD_PARTITIONS
> +#define OF_FLASH_PARTS(info) ((info)->parts)
> +
> static int parse_obsolete_partitions(struct of_device *dev,
> - struct physmap_flash_info *info,
> + struct of_flash *info,
> struct device_node *dp)
> {
> int i, plen, nr_parts;
> @@ -56,11 +53,9 @@ static int parse_obsolete_partitions(str
>
> nr_parts = plen / sizeof(part[0]);
>
> - info->parts = kzalloc(nr_parts * sizeof(struct mtd_partition),
> GFP_KERNEL);
> - if (!info->parts) {
> - printk(KERN_ERR "Can't allocate the flash partition data!\n");
> + info->parts = kzalloc(nr_parts * sizeof(*info->parts), GFP_KERNEL);
> + if (!info->parts)
> return -ENOMEM;
You dropped the printk here. Any particular reason why?
> - }
>
> names = of_get_property(dp, "partition-names", &plen);
>
> @@ -86,8 +81,8 @@ static int parse_obsolete_partitions(str
> return nr_parts;
> }
>
> -static int __devinit process_partitions(struct physmap_flash_info *info,
> - struct of_device *dev)
> +static int __devinit parse_partitions(struct of_flash *info,
> + struct of_device *dev)
> {
> const char *partname;
> static const char *part_probe_types[]
> @@ -109,87 +104,68 @@ static int __devinit process_partitions(
> for (pp = dp->child; pp; pp = pp->sibling)
> nr_parts++;
>
> - if (nr_parts) {
> - info->parts = kzalloc(nr_parts * sizeof(struct mtd_partition),
> - GFP_KERNEL);
> - if (!info->parts) {
> - printk(KERN_ERR "Can't allocate the flash partition
> data!\n");
> - return -ENOMEM;
> - }
> + if (nr_parts == 0)
> + return parse_obsolete_partitions(dev, info, dp);
You reintroduced the optional partitions bug I fixed with:
http://git.infradead.org/?p=users/jwboyer/powerpc.git;a=commit;h=7cafc8f8c89d1f49039b7c345ca832fbd2d1e639
josh
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
