Message ID | 20181008201027.17952-1-boris.brezillon@bootlin.com |
---|---|
Headers | show |
Series | mtd: maps: physmap cleanups | expand |
Hi Boris On Mon, Oct 8, 2018 at 10:10 PM Boris Brezillon <boris.brezillon@bootlin.com> wrote: > > There's no real reason to have two separate driver for the DT and pdata > case. Just do what we do everywhere else and handle DT and pdata > parsing in the same driver. > > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> > --- > drivers/mtd/maps/Kconfig | 4 +- > drivers/mtd/maps/Makefile | 7 +- > drivers/mtd/maps/physmap-core.c | 260 +++++++++++++++++++++++--- > drivers/mtd/maps/physmap_of_core.c | 368 ------------------------------------- > 4 files changed, 239 insertions(+), 400 deletions(-) > delete mode 100644 drivers/mtd/maps/physmap_of_core.c > > diff --git a/drivers/mtd/maps/Kconfig b/drivers/mtd/maps/Kconfig > index afb36bff13a7..5bffebacce86 100644 > --- a/drivers/mtd/maps/Kconfig > +++ b/drivers/mtd/maps/Kconfig > @@ -66,8 +66,8 @@ config MTD_PHYSMAP_BANKWIDTH > used internally by the CFI drivers. > > config MTD_PHYSMAP_OF > - tristate "Memory device in physical memory map based on OF description" > - depends on OF && (MTD_CFI || MTD_JEDECPROBE || MTD_ROM || MTD_RAM) > + bool "Memory device in physical memory map based on OF description" > + depends on OF && MTD_PHYSMAP > help > This provides a 'mapping' driver which allows the NOR Flash, ROM > and RAM driver code to communicate with chips which are mapped > diff --git a/drivers/mtd/maps/Makefile b/drivers/mtd/maps/Makefile > index 2574909edffd..ad32b185a120 100644 > --- a/drivers/mtd/maps/Makefile > +++ b/drivers/mtd/maps/Makefile > @@ -18,13 +18,10 @@ obj-$(CONFIG_MTD_CK804XROM) += ck804xrom.o > obj-$(CONFIG_MTD_TSUNAMI) += tsunami_flash.o > obj-$(CONFIG_MTD_PXA2XX) += pxa2xx-flash.o > physmap-objs-y += physmap-core.o > +physmap-objs-$(CONFIG_MTD_PHYSMAP_OF_VERSATILE) += physmap_of_versatile.o > +physmap-objs-$(CONFIG_MTD_PHYSMAP_OF_GEMINI) += physmap_of_gemini.o > physmap-objs := $(physmap-objs-y) > obj-$(CONFIG_MTD_PHYSMAP) += physmap.o > -physmap_of-objs-y += physmap_of_core.o > -physmap_of-objs-$(CONFIG_MTD_PHYSMAP_OF_VERSATILE) += physmap_of_versatile.o > -physmap_of-objs-$(CONFIG_MTD_PHYSMAP_OF_GEMINI) += physmap_of_gemini.o > -physmap_of-objs := $(physmap_of-objs-y) > -obj-$(CONFIG_MTD_PHYSMAP_OF) += physmap_of.o > obj-$(CONFIG_MTD_PISMO) += pismo.o > obj-$(CONFIG_MTD_PMC_MSP_EVM) += pmcmsp-flash.o > obj-$(CONFIG_MTD_PCMCIA) += pcmciamtd.o > diff --git a/drivers/mtd/maps/physmap-core.c b/drivers/mtd/maps/physmap-core.c > index e27051bc5dc6..7a50ff9ef812 100644 > --- a/drivers/mtd/maps/physmap-core.c > +++ b/drivers/mtd/maps/physmap-core.c > @@ -6,6 +6,13 @@ > * Author: Jun Sun, jsun@mvista.com or jsun@junsun.net > * > * 031022 - [jsun] add run-time configure and partition setup > + * > + * Device tree support: > + * Copyright (C) 2006 MontaVista Software Inc. > + * Author: Vitaly Wool <vwool@ru.mvista.com> > + * > + * Revised to handle newer style flash binding by: > + * Copyright (C) 2007 David Gibson, IBM Corporation. > */ > > #include <linux/module.h> > @@ -20,7 +27,12 @@ > #include <linux/mtd/partitions.h> > #include <linux/mtd/physmap.h> > #include <linux/mtd/concat.h> > +#include <linux/mtd/cfi_endian.h> > #include <linux/io.h> > +#include <linux/of_device.h> > + > +#include "physmap_of_gemini.h" > +#include "physmap_of_versatile.h" > > struct physmap_flash_info { > unsigned int nmaps; > @@ -29,6 +41,10 @@ struct physmap_flash_info { > struct map_info *maps; > spinlock_t vpp_lock; > int vpp_refcnt; > + const char *probe_type; > + const char * const *part_types; > + unsigned int nparts; > + const struct mtd_partition *parts; > }; > > static int physmap_flash_remove(struct platform_device *dev) > @@ -41,8 +57,6 @@ static int physmap_flash_remove(struct platform_device *dev) > if (!info) > return 0; > > - physmap_data = dev_get_platdata(&dev->dev); > - > if (info->cmtd) { > err = mtd_device_unregister(info->cmtd); > if (err) > @@ -57,7 +71,8 @@ static int physmap_flash_remove(struct platform_device *dev) > map_destroy(info->mtds[i]); > } > > - if (physmap_data->exit) > + physmap_data = dev_get_platdata(&dev->dev); > + if (physmap_data && physmap_data->exit) > physmap_data->exit(dev); > > return 0; > @@ -89,6 +104,172 @@ static void physmap_set_vpp(struct map_info *map, int state) > spin_unlock_irqrestore(&info->vpp_lock, flags); > } > > +#if IS_ENABLED(CONFIG_MTD_PHYSMAP_OF) > +static const struct of_device_id of_flash_match[] = { > + { > + .compatible = "cfi-flash", > + .data = "cfi_probe", > + }, > + { > + /* > + * FIXME: JEDEC chips can't be safely and reliably > + * probed, although the mtd code gets it right in > + * practice most of the time. We should use the > + * vendor and device ids specified by the binding to > + * bypass the heuristic probe code, but the mtd layer > + * provides, at present, no interface for doing so > + * :(. > + */ > + .compatible = "jedec-flash", > + .data = "jedec_probe", > + }, > + { > + .compatible = "mtd-ram", > + .data = "map_ram", > + }, > + { > + .compatible = "mtd-rom", > + .data = "map_rom", > + }, > + { > + .type = "rom", > + .compatible = "direct-mapped" > + }, > + { /* sentinel */ }, > +}; > +MODULE_DEVICE_TABLE(of, of_flash_match); > + > +static const char * const of_default_part_probes[] = { > + "cmdlinepart", "RedBoot", "ofpart", "ofoldpart", NULL > +}; > + > +static const char * const *of_get_part_probes(struct platform_device *dev) > +{ > + struct device_node *dp = dev->dev.of_node; > + const char **res; > + int count; > + > + count = of_property_count_strings(dp, "linux,part-probe"); > + if (count < 0) > + return of_default_part_probes; > + > + res = devm_kcalloc(&dev->dev, count + 1, sizeof(*res), GFP_KERNEL); > + if (!res) > + return NULL; > + > + count = of_property_read_string_array(dp, "linux,part-probe", res, > + count); > + if (count < 0) > + return NULL; > + > + return res; > +} > + > +static const char *of_select_probe_type(struct platform_device *dev) > +{ > + struct device_node *dp = dev->dev.of_node; > + const struct of_device_id *match; > + const char *probe_type; > + > + match = of_match_device(of_flash_match, &dev->dev); > + probe_type = match->data; > + if (probe_type) > + return probe_type; > + > + dev_warn(&dev->dev, > + "Device tree uses obsolete \"direct-mapped\" flash binding\n"); > + > + of_property_read_string(dp, "probe-type", &probe_type); > + if (!probe_type) > + return NULL; > + > + if (!strcmp(probe_type, "CFI")) { > + probe_type = "cfi_probe"; > + } else if (!strcmp(probe_type, "JEDEC")) { > + probe_type = "jedec_probe"; > + } else if (!strcmp(probe_type, "ROM")) { > + probe_type = "map_rom"; > + } else { > + dev_warn(&dev->dev, > + "obsolete_probe: don't know probe type '%s', mapping as rom\n", > + probe_type); > + probe_type = "map_rom"; > + } > + > + return probe_type; > +} > + > +static int physmap_flash_of_init(struct platform_device *dev) > +{ > + struct physmap_flash_info *info = platform_get_drvdata(dev); > + struct device_node *dp = dev->dev.of_node; > + const char *mtd_name = NULL; > + int err, swap = 0; > + bool map_indirect; > + unsigned int i; > + u32 bankwidth; > + > + if (!dp) > + return -EINVAL; > + > + info->probe_type = of_select_probe_type(dev); > + > + info->part_types = of_get_part_probes(dev); > + if (!info->part_types) > + return -ENOMEM; > + > + of_property_read_string(dp, "linux,mtd-name", &mtd_name); > + > + map_indirect = of_property_read_bool(dp, "no-unaligned-direct-access"); > + > + err = of_property_read_u32(dp, "bank-width", &bankwidth); > + if (err) { > + dev_err(&dev->dev, "Can't get bank width from device tree\n"); > + return err; > + } > + > + if (of_property_read_bool(dp, "big-endian")) > + swap = CFI_BIG_ENDIAN; > + else if (of_property_read_bool(dp, "little-endian")) > + swap = CFI_LITTLE_ENDIAN; > + > + for (i = 0; i < info->nmaps; i++) { > + info->maps[i].name = mtd_name; > + info->maps[i].swap = swap; > + info->maps[i].bankwidth = bankwidth; > + info->maps[i].device_node = dp; > + > + err = of_flash_probe_gemini(dev, dp, &info->maps[i]); > + if (err) > + return err; > + > + err = of_flash_probe_versatile(dev, dp, &info->maps[i]); > + if (err) > + return err; > + > + /* > + * On some platforms (e.g. MPC5200) a direct 1:1 mapping > + * may cause problems with JFFS2 usage, as the local bus (LPB) > + * doesn't support unaligned accesses as implemented in the > + * JFFS2 code via memcpy(). By setting NO_XIP, the > + * flash will not be exposed directly to the MTD users > + * (e.g. JFFS2) any more. > + */ > + if (map_indirect) > + info->maps[i].phys = NO_XIP; > + } > + > + return 0; > +} > +#else /* IS_ENABLED(CONFIG_MTD_PHYSMAP_OF) */ > +#define of_flash_match NULL > + > +static int physmap_flash_of_init(struct platform_device *dev) > +{ > + return -ENOTSUPP; > +} > +#endif /* IS_ENABLED(CONFIG_MTD_PHYSMAP_OF) */ > + > static const char * const rom_probe_types[] = { > "cfi_probe", "jedec_probe", "qinfo_probe", "map_rom", NULL > }; > @@ -97,18 +278,46 @@ static const char * const part_probe_types[] = { > "cmdlinepart", "RedBoot", "afs", NULL > }; > > -static int physmap_flash_probe(struct platform_device *dev) > +static int physmap_flash_pdata_init(struct platform_device *dev) > { > + struct physmap_flash_info *info = platform_get_drvdata(dev); > struct physmap_flash_data *physmap_data; > + unsigned int i; > + int err; > + > + physmap_data = dev_get_platdata(&dev->dev); > + if (!physmap_data) > + return -EINVAL; > + > + info->probe_type = physmap_data->probe_type; > + info->part_types = physmap_data->part_probe_types ? : part_probe_types; > + info->parts = physmap_data->parts; > + info->nparts = physmap_data->nr_parts; > + > + if (physmap_data->init) { > + err = physmap_data->init(dev); > + if (err) > + return err; > + } > + > + for (i = 0; i < info->nmaps; i++) { > + info->maps[i].bankwidth = physmap_data->width; > + info->maps[i].pfow_base = physmap_data->pfow_base; > + info->maps[i].set_vpp = physmap_set_vpp; > + } > + > + return 0; > +} > + > +static int physmap_flash_probe(struct platform_device *dev) > +{ > struct physmap_flash_info *info; > const char * const *probe_type; > - const char * const *part_types; > int err = 0; > int i; > > - physmap_data = dev_get_platdata(&dev->dev); > - if (!physmap_data) > - return -ENODEV; > + if (!dev->dev.of_node && dev_get_platdata(&dev->dev)) > + return -EINVAL; Maybe you want: if (!dev->dev.of_node && !dev_get_platdata(&dev->dev)) > > info = devm_kzalloc(&dev->dev, sizeof(*info), GFP_KERNEL); > if (!info) > @@ -132,14 +341,15 @@ static int physmap_flash_probe(struct platform_device *dev) > if (!info->mtds) > return -ENOMEM; > > - if (physmap_data->init) { > - err = physmap_data->init(dev); > - if (err) > - goto err_out; > - } > - > platform_set_drvdata(dev, info); > > + err = physmap_flash_of_init(dev); > + if (err) > + err = physmap_flash_pdata_init(dev); > + Maybe replace this with: if (dev->dev.of_node) err = physmap_flash_of_init(dev); else err = physmap_flash_pdata_init(dev); Otherwise we are hiding the returncode from of_init, which might be defered probing. > + if (err) > + return err; > + > for (i = 0; i < info->nmaps; i++) { > struct resource *res; > > @@ -154,22 +364,22 @@ static int physmap_flash_probe(struct platform_device *dev) > res); > > info->maps[i].name = dev_name(&dev->dev); > - info->maps[i].phys = res->start; > + > + if (!info->maps[i].phys) > + info->maps[i].phys = res->start; > + > info->maps[i].size = resource_size(res); > - info->maps[i].bankwidth = physmap_data->width; > - info->maps[i].set_vpp = physmap_set_vpp; > - info->maps[i].pfow_base = physmap_data->pfow_base; > info->maps[i].map_priv_1 = (unsigned long)dev; > > simple_map_init(&info->maps[i]); > > probe_type = rom_probe_types; > - if (!physmap_data->probe_type) { > + if (!info->probe_type) { > for (; !info->mtds[i] && *probe_type; probe_type++) > info->mtds[i] = do_map_probe(*probe_type, > &info->maps[i]); > } else { > - info->mtds[i] = do_map_probe(physmap_data->probe_type, > + info->mtds[i] = do_map_probe(info->probe_type, > &info->maps[i]); > } > > @@ -197,11 +407,9 @@ static int physmap_flash_probe(struct platform_device *dev) > > spin_lock_init(&info->vpp_lock); > > - part_types = physmap_data->part_probe_types ? : part_probe_types; > - > - err = mtd_device_parse_register(info->cmtd, part_types, NULL, > - physmap_data->parts, > - physmap_data->nr_parts); > + mtd_set_of_node(info->cmtd, dev->dev.of_node); > + err = mtd_device_parse_register(info->cmtd, info->part_types, NULL, > + info->parts, info->nparts); > if (err) > goto err_out; > > @@ -232,6 +440,7 @@ static struct platform_driver physmap_flash_driver = { > .shutdown = physmap_flash_shutdown, > .driver = { > .name = "physmap-flash", > + .of_match_table = of_flash_match, > }, > }; > > @@ -286,6 +495,7 @@ module_exit(physmap_exit); > > MODULE_LICENSE("GPL"); > MODULE_AUTHOR("David Woodhouse <dwmw2@infradead.org>"); > +MODULE_AUTHOR("Vitaly Wool <vwool@ru.mvista.com>"); > MODULE_DESCRIPTION("Generic configurable MTD map driver"); > > /* legacy platform drivers can't hotplug or coldplg */ > diff --git a/drivers/mtd/maps/physmap_of_core.c b/drivers/mtd/maps/physmap_of_core.c > deleted file mode 100644 > index ece605d78c21..000000000000 > --- a/drivers/mtd/maps/physmap_of_core.c > +++ /dev/null > @@ -1,368 +0,0 @@ > -/* > - * Flash mappings described by the OF (or flattened) device tree > - * > - * Copyright (C) 2006 MontaVista Software Inc. > - * Author: Vitaly Wool <vwool@ru.mvista.com> > - * > - * Revised to handle newer style flash binding by: > - * Copyright (C) 2007 David Gibson, IBM Corporation. > - * > - * This program is free software; you can redistribute it and/or modify it > - * under the terms of the GNU General Public License as published by the > - * Free Software Foundation; either version 2 of the License, or (at your > - * option) any later version. > - */ > - > -#include <linux/module.h> > -#include <linux/types.h> > -#include <linux/device.h> > -#include <linux/mtd/mtd.h> > -#include <linux/mtd/map.h> > -#include <linux/mtd/partitions.h> > -#include <linux/mtd/concat.h> > -#include <linux/mtd/cfi_endian.h> > -#include <linux/of.h> > -#include <linux/of_address.h> > -#include <linux/of_platform.h> > -#include <linux/slab.h> > -#include "physmap_of_gemini.h" > -#include "physmap_of_versatile.h" > - > -struct of_flash_list { > - struct mtd_info *mtd; > - struct map_info map; > -}; > - > -struct of_flash { > - struct mtd_info *cmtd; > - int list_size; /* number of elements in of_flash_list */ > - struct of_flash_list list[0]; > -}; > - > -static int of_flash_remove(struct platform_device *dev) > -{ > - struct of_flash *info; > - int i; > - > - info = dev_get_drvdata(&dev->dev); > - if (!info) > - return 0; > - dev_set_drvdata(&dev->dev, NULL); > - > - if (info->cmtd) { > - mtd_device_unregister(info->cmtd); > - if (info->cmtd != info->list[0].mtd) > - mtd_concat_destroy(info->cmtd); > - } > - > - for (i = 0; i < info->list_size; i++) > - if (info->list[i].mtd) > - map_destroy(info->list[i].mtd); > - > - return 0; > -} > - > -static const char * const rom_probe_types[] = { > - "cfi_probe", "jedec_probe", "map_rom" }; > - > -/* Helper function to handle probing of the obsolete "direct-mapped" > - * compatible binding, which has an extra "probe-type" property > - * describing the type of flash probe necessary. */ > -static struct mtd_info *obsolete_probe(struct platform_device *dev, > - struct map_info *map) > -{ > - struct device_node *dp = dev->dev.of_node; > - const char *of_probe; > - struct mtd_info *mtd; > - int i; > - > - dev_warn(&dev->dev, "Device tree uses obsolete \"direct-mapped\" " > - "flash binding\n"); > - > - of_probe = of_get_property(dp, "probe-type", NULL); > - if (!of_probe) { > - for (i = 0; i < ARRAY_SIZE(rom_probe_types); i++) { > - mtd = do_map_probe(rom_probe_types[i], map); > - if (mtd) > - return mtd; > - } > - return NULL; > - } else if (strcmp(of_probe, "CFI") == 0) { > - return do_map_probe("cfi_probe", map); > - } else if (strcmp(of_probe, "JEDEC") == 0) { > - return do_map_probe("jedec_probe", map); > - } else { > - if (strcmp(of_probe, "ROM") != 0) > - dev_warn(&dev->dev, "obsolete_probe: don't know probe " > - "type '%s', mapping as rom\n", of_probe); > - return do_map_probe("map_rom", map); > - } > -} > - > -/* When partitions are set we look for a linux,part-probe property which > - specifies the list of partition probers to use. If none is given then the > - default is use. These take precedence over other device tree > - information. */ > -static const char * const part_probe_types_def[] = { > - "cmdlinepart", "RedBoot", "ofpart", "ofoldpart", NULL }; > - > -static const char * const *of_get_probes(struct device_node *dp) > -{ > - const char **res; > - int count; > - > - count = of_property_count_strings(dp, "linux,part-probe"); > - if (count < 0) > - return part_probe_types_def; > - > - res = kcalloc(count + 1, sizeof(*res), GFP_KERNEL); > - if (!res) > - return NULL; > - > - count = of_property_read_string_array(dp, "linux,part-probe", res, > - count); > - if (count < 0) > - return NULL; > - > - return res; > -} > - > -static void of_free_probes(const char * const *probes) > -{ > - if (probes != part_probe_types_def) > - kfree(probes); > -} > - > -static const struct of_device_id of_flash_match[]; > -static int of_flash_probe(struct platform_device *dev) > -{ > - const char * const *part_probe_types; > - const struct of_device_id *match; > - struct device_node *dp = dev->dev.of_node; > - struct resource res; > - struct of_flash *info; > - const char *probe_type; > - const __be32 *width; > - int err; > - int i; > - int count; > - const __be32 *p; > - int reg_tuple_size; > - struct mtd_info **mtd_list = NULL; > - resource_size_t res_size; > - bool map_indirect; > - const char *mtd_name = NULL; > - > - match = of_match_device(of_flash_match, &dev->dev); > - if (!match) > - return -EINVAL; > - probe_type = match->data; > - > - reg_tuple_size = (of_n_addr_cells(dp) + of_n_size_cells(dp)) * sizeof(u32); > - > - of_property_read_string(dp, "linux,mtd-name", &mtd_name); > - > - /* > - * Get number of "reg" tuples. Scan for MTD devices on area's > - * described by each "reg" region. This makes it possible (including > - * the concat support) to support the Intel P30 48F4400 chips which > - * consists internally of 2 non-identical NOR chips on one die. > - */ > - p = of_get_property(dp, "reg", &count); > - if (!p || count % reg_tuple_size != 0) { > - dev_err(&dev->dev, "Malformed reg property on %pOF\n", > - dev->dev.of_node); > - err = -EINVAL; > - goto err_flash_remove; > - } > - count /= reg_tuple_size; > - > - map_indirect = of_property_read_bool(dp, "no-unaligned-direct-access"); > - > - err = -ENOMEM; > - info = devm_kzalloc(&dev->dev, > - sizeof(struct of_flash) + > - sizeof(struct of_flash_list) * count, GFP_KERNEL); > - if (!info) > - goto err_flash_remove; > - > - dev_set_drvdata(&dev->dev, info); > - > - mtd_list = kcalloc(count, sizeof(*mtd_list), GFP_KERNEL); > - if (!mtd_list) > - goto err_flash_remove; > - > - for (i = 0; i < count; i++) { > - err = -ENXIO; > - if (of_address_to_resource(dp, i, &res)) { > - /* > - * Continue with next register tuple if this > - * one is not mappable > - */ > - continue; > - } > - > - dev_dbg(&dev->dev, "of_flash device: %pR\n", &res); > - > - err = -EBUSY; > - res_size = resource_size(&res); > - info->list[i].map.virt = devm_ioremap_resource(&dev->dev, &res); > - if (IS_ERR(info->list[i].map.virt)) { > - err = PTR_ERR(info->list[i].map.virt); > - goto err_out; > - } > - > - err = -ENXIO; > - width = of_get_property(dp, "bank-width", NULL); > - if (!width) { > - dev_err(&dev->dev, "Can't get bank width from device" > - " tree\n"); > - goto err_out; > - } > - > - info->list[i].map.name = mtd_name ?: dev_name(&dev->dev); > - info->list[i].map.phys = res.start; > - info->list[i].map.size = res_size; > - info->list[i].map.bankwidth = be32_to_cpup(width); > - info->list[i].map.device_node = dp; > - > - if (of_property_read_bool(dp, "big-endian")) > - info->list[i].map.swap = CFI_BIG_ENDIAN; > - else if (of_property_read_bool(dp, "little-endian")) > - info->list[i].map.swap = CFI_LITTLE_ENDIAN; > - > - err = of_flash_probe_gemini(dev, dp, &info->list[i].map); > - if (err) > - goto err_out; > - err = of_flash_probe_versatile(dev, dp, &info->list[i].map); > - if (err) > - goto err_out; > - > - simple_map_init(&info->list[i].map); > - > - /* > - * On some platforms (e.g. MPC5200) a direct 1:1 mapping > - * may cause problems with JFFS2 usage, as the local bus (LPB) > - * doesn't support unaligned accesses as implemented in the > - * JFFS2 code via memcpy(). By setting NO_XIP, the > - * flash will not be exposed directly to the MTD users > - * (e.g. JFFS2) any more. > - */ > - if (map_indirect) > - info->list[i].map.phys = NO_XIP; > - > - if (probe_type) { > - info->list[i].mtd = do_map_probe(probe_type, > - &info->list[i].map); > - } else { > - info->list[i].mtd = obsolete_probe(dev, > - &info->list[i].map); > - } > - > - /* Fall back to mapping region as ROM */ > - if (!info->list[i].mtd) { > - dev_warn(&dev->dev, > - "do_map_probe() failed for type %s\n", > - probe_type); > - > - info->list[i].mtd = do_map_probe("map_rom", > - &info->list[i].map); > - } > - mtd_list[i] = info->list[i].mtd; > - > - err = -ENXIO; > - if (!info->list[i].mtd) { > - dev_err(&dev->dev, "do_map_probe() failed\n"); > - goto err_out; > - } else { > - info->list_size++; > - } > - info->list[i].mtd->dev.parent = &dev->dev; > - } > - > - err = 0; > - info->cmtd = NULL; > - if (info->list_size == 1) { > - info->cmtd = info->list[0].mtd; > - } else if (info->list_size > 1) { > - /* > - * We detected multiple devices. Concatenate them together. > - */ > - info->cmtd = mtd_concat_create(mtd_list, info->list_size, > - dev_name(&dev->dev)); > - } > - if (info->cmtd == NULL) > - err = -ENXIO; > - > - if (err) > - goto err_out; > - > - info->cmtd->dev.parent = &dev->dev; > - mtd_set_of_node(info->cmtd, dp); > - part_probe_types = of_get_probes(dp); > - if (!part_probe_types) { > - err = -ENOMEM; > - goto err_out; > - } > - mtd_device_parse_register(info->cmtd, part_probe_types, NULL, > - NULL, 0); > - of_free_probes(part_probe_types); > - > - kfree(mtd_list); > - > - return 0; > - > -err_out: > - kfree(mtd_list); > -err_flash_remove: > - of_flash_remove(dev); > - > - return err; > -} > - > -static const struct of_device_id of_flash_match[] = { > - { > - .compatible = "cfi-flash", > - .data = (void *)"cfi_probe", > - }, > - { > - /* FIXME: JEDEC chips can't be safely and reliably > - * probed, although the mtd code gets it right in > - * practice most of the time. We should use the > - * vendor and device ids specified by the binding to > - * bypass the heuristic probe code, but the mtd layer > - * provides, at present, no interface for doing so > - * :(. */ > - .compatible = "jedec-flash", > - .data = (void *)"jedec_probe", > - }, > - { > - .compatible = "mtd-ram", > - .data = (void *)"map_ram", > - }, > - { > - .compatible = "mtd-rom", > - .data = (void *)"map_rom", > - }, > - { > - .type = "rom", > - .compatible = "direct-mapped" > - }, > - { }, > -}; > -MODULE_DEVICE_TABLE(of, of_flash_match); > - > -static struct platform_driver of_flash_driver = { > - .driver = { > - .name = "of-flash", > - .of_match_table = of_flash_match, > - }, > - .probe = of_flash_probe, > - .remove = of_flash_remove, > -}; > - > -module_platform_driver(of_flash_driver); > - > -MODULE_LICENSE("GPL"); > -MODULE_AUTHOR("Vitaly Wool <vwool@ru.mvista.com>"); > -MODULE_DESCRIPTION("Device tree based MTD map driver"); > -- > 2.14.1 >
Hi Boris Maybe we want to leave the pdata example. /** * The platform resource layout expected looks something like: * struct mtd_partition partitions[] = { ... }; * struct physmap_flash_data flash_data = ..... On Mon, Oct 8, 2018 at 10:10 PM Boris Brezillon <boris.brezillon@bootlin.com> wrote: > > Controlling some MSB address lines using GPIOs is just a small > deviation of the generic physmap logic, and merging those two drivers > allows us to share most of the probe logic, which is a good thing. > > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> > --- > drivers/mtd/maps/Kconfig | 19 ++- > drivers/mtd/maps/Makefile | 1 - > drivers/mtd/maps/gpio-addr-flash.c | 281 ------------------------------------- > drivers/mtd/maps/physmap-core.c | 150 +++++++++++++++++++- > 4 files changed, 157 insertions(+), 294 deletions(-) > delete mode 100644 drivers/mtd/maps/gpio-addr-flash.c > > diff --git a/drivers/mtd/maps/Kconfig b/drivers/mtd/maps/Kconfig > index 5bffebacce86..4fd13d76b7b5 100644 > --- a/drivers/mtd/maps/Kconfig > +++ b/drivers/mtd/maps/Kconfig > @@ -94,6 +94,15 @@ config MTD_PHYSMAP_OF_GEMINI > platforms, some detection and setting up parallel mode on the > external interface. > > +config MTD_PHYSMAP_GPIO_ADDR > + bool "GPIO-assisted Flash Chip Support" > + depends on MTD_PHYSMAP > + depends on GPIOLIB || COMPILE_TEST > + depends on MTD_COMPLEX_MAPPINGS > + help > + Extend the physmap driver to allow flashes to be partially > + physically addressed and assisted by GPIOs. > + > config MTD_PMC_MSP_EVM > tristate "CFI Flash device mapped on PMC-Sierra MSP" > depends on PMC_MSP && MTD_CFI > @@ -334,16 +343,6 @@ config MTD_PCMCIA_ANONYMOUS > > If unsure, say N. > > -config MTD_GPIO_ADDR > - tristate "GPIO-assisted Flash Chip Support" > - depends on GPIOLIB || COMPILE_TEST > - depends on MTD_COMPLEX_MAPPINGS > - help > - Map driver which allows flashes to be partially physically addressed > - and assisted by GPIOs. > - > - If compiled as a module, it will be called gpio-addr-flash. > - > config MTD_UCLINUX > bool "Generic uClinux RAM/ROM filesystem support" > depends on (MTD_RAM=y || MTD_ROM=y) && (!MMU || COLDFIRE) > diff --git a/drivers/mtd/maps/Makefile b/drivers/mtd/maps/Makefile > index ad32b185a120..acec0fbfa18d 100644 > --- a/drivers/mtd/maps/Makefile > +++ b/drivers/mtd/maps/Makefile > @@ -43,6 +43,5 @@ obj-$(CONFIG_MTD_PLATRAM) += plat-ram.o > obj-$(CONFIG_MTD_INTEL_VR_NOR) += intel_vr_nor.o > obj-$(CONFIG_MTD_RBTX4939) += rbtx4939-flash.o > obj-$(CONFIG_MTD_VMU) += vmu-flash.o > -obj-$(CONFIG_MTD_GPIO_ADDR) += gpio-addr-flash.o > obj-$(CONFIG_MTD_LATCH_ADDR) += latch-addr-flash.o > obj-$(CONFIG_MTD_LANTIQ) += lantiq-flash.o > diff --git a/drivers/mtd/maps/gpio-addr-flash.c b/drivers/mtd/maps/gpio-addr-flash.c > deleted file mode 100644 > index a20e85aa770e..000000000000 > --- a/drivers/mtd/maps/gpio-addr-flash.c > +++ /dev/null > @@ -1,281 +0,0 @@ > -/* > - * drivers/mtd/maps/gpio-addr-flash.c > - * > - * Handle the case where a flash device is mostly addressed using physical > - * line and supplemented by GPIOs. This way you can hook up say a 8MiB flash > - * to a 2MiB memory range and use the GPIOs to select a particular range. > - * > - * Copyright © 2000 Nicolas Pitre <nico@cam.org> > - * Copyright © 2005-2009 Analog Devices Inc. > - * > - * Enter bugs at http://blackfin.uclinux.org/ > - * > - * Licensed under the GPL-2 or later. > - */ > - > -#include <linux/gpio.h> > -#include <linux/gpio/consumer.h> > -#include <linux/io.h> > -#include <linux/kernel.h> > -#include <linux/module.h> > -#include <linux/mtd/mtd.h> > -#include <linux/mtd/map.h> > -#include <linux/mtd/partitions.h> > -#include <linux/mtd/physmap.h> > -#include <linux/platform_device.h> > -#include <linux/slab.h> > -#include <linux/types.h> > - > -#define win_mask(x) ((BIT(x)) - 1) > - > -#define DRIVER_NAME "gpio-addr-flash" > - > -/** > - * struct async_state - keep GPIO flash state > - * @mtd: MTD state for this mapping > - * @map: MTD map state for this flash > - * @gpios: Struct containing the array of GPIO descriptors > - * @gpio_values: cached GPIO values > - * @win_order: dedicated memory size (if no GPIOs) > - */ > -struct async_state { > - struct mtd_info *mtd; > - struct map_info map; > - struct gpio_descs *gpios; > - unsigned int gpio_values; > - unsigned int win_order; > -}; > -#define gf_map_info_to_state(mi) ((struct async_state *)(mi)->map_priv_1) > - > -/** > - * gf_set_gpios() - set GPIO address lines to access specified flash offset > - * @state: GPIO flash state > - * @ofs: desired offset to access > - * > - * Rather than call the GPIO framework every time, cache the last-programmed > - * value. This speeds up sequential accesses (which are by far the most common > - * type). > - */ > -static void gf_set_gpios(struct async_state *state, unsigned long ofs) > -{ > - int i; > - > - ofs >>= state->win_order; > - > - if (ofs == state->gpio_values) > - return; > - > - for (i = 0; i < state->gpios->ndescs; i++) { > - if ((ofs & BIT(i)) == (state->gpio_values & BIT(i))) > - continue; > - > - gpiod_set_value(state->gpios->desc[i], !!(ofs & BIT(i))); > - } > - > - state->gpio_values = ofs; > -} > - > -/** > - * gf_read() - read a word at the specified offset > - * @map: MTD map state > - * @ofs: desired offset to read > - */ > -static map_word gf_read(struct map_info *map, unsigned long ofs) > -{ > - struct async_state *state = gf_map_info_to_state(map); > - uint16_t word; > - map_word test; > - > - gf_set_gpios(state, ofs); > - > - word = readw(map->virt + (ofs & win_mask(state->win_order))); > - test.x[0] = word; > - return test; > -} > - > -/** > - * gf_copy_from() - copy a chunk of data from the flash > - * @map: MTD map state > - * @to: memory to copy to > - * @from: flash offset to copy from > - * @len: how much to copy > - * > - * The "from" region may straddle more than one window, so toggle the GPIOs for > - * each window region before reading its data. > - */ > -static void gf_copy_from(struct map_info *map, void *to, unsigned long from, ssize_t len) > -{ > - struct async_state *state = gf_map_info_to_state(map); > - > - int this_len; > - > - while (len) { > - this_len = from & win_mask(state->win_order); > - this_len = BIT(state->win_order) - this_len; > - this_len = min_t(int, len, this_len); > - > - gf_set_gpios(state, from); > - memcpy_fromio(to, > - map->virt + (from & win_mask(state->win_order)), > - this_len); > - len -= this_len; > - from += this_len; > - to += this_len; > - } > -} > - > -/** > - * gf_write() - write a word at the specified offset > - * @map: MTD map state > - * @ofs: desired offset to write > - */ > -static void gf_write(struct map_info *map, map_word d1, unsigned long ofs) > -{ > - struct async_state *state = gf_map_info_to_state(map); > - uint16_t d; > - > - gf_set_gpios(state, ofs); > - > - d = d1.x[0]; > - writew(d, map->virt + (ofs & win_mask(state->win_order))); > -} > - > -/** > - * gf_copy_to() - copy a chunk of data to the flash > - * @map: MTD map state > - * @to: flash offset to copy to > - * @from: memory to copy from > - * @len: how much to copy > - * > - * See gf_copy_from() caveat. > - */ > -static void gf_copy_to(struct map_info *map, unsigned long to, > - const void *from, ssize_t len) > -{ > - struct async_state *state = gf_map_info_to_state(map); > - > - int this_len; > - > - while (len) { > - this_len = to & win_mask(state->win_order); > - this_len = BIT(state->win_order) - this_len; > - this_len = min_t(int, len, this_len); > - > - gf_set_gpios(state, to); > - memcpy_toio(map->virt + (to & win_mask(state->win_order)), > - from, len); > - > - len -= this_len; > - to += this_len; > - from += this_len; > - } > -} > - > -static const char * const part_probe_types[] = { > - "cmdlinepart", "RedBoot", NULL }; > - > -/** > - * gpio_flash_probe() - setup a mapping for a GPIO assisted flash > - * @pdev: platform device > - * > - * The platform resource layout expected looks something like: > - * struct mtd_partition partitions[] = { ... }; > - * struct physmap_flash_data flash_data = { ... }; > - * static struct gpiod_lookup_table addr_flash_gpios = { > - * .dev_id = "gpio-addr-flash.0", > - * .table = { > - * GPIO_LOOKUP_IDX("gpio.0", 15, "addr", 0, GPIO_ACTIVE_HIGH), > - * GPIO_LOOKUP_IDX("gpio.0", 16, "addr", 1, GPIO_ACTIVE_HIGH), > - * ); > - * }; > - * gpiod_add_lookup_table(&addr_flash_gpios); > - * > - * struct resource flash_resource[] = { > - * { > - * .name = "cfi_probe", > - * .start = 0x20000000, > - * .end = 0x201fffff, > - * .flags = IORESOURCE_MEM, > - * }, > - * }; > - * struct platform_device flash_device = { > - * .name = "gpio-addr-flash", > - * .dev = { .platform_data = &flash_data, }, > - * .num_resources = ARRAY_SIZE(flash_resource), > - * .resource = flash_resource, > - * ... > - * }; > - */ > -static int gpio_flash_probe(struct platform_device *pdev) > -{ > - struct physmap_flash_data *pdata; > - struct resource *memory; > - struct async_state *state; > - > - pdata = dev_get_platdata(&pdev->dev); > - memory = platform_get_resource(pdev, IORESOURCE_MEM, 0); > - > - if (!memory) > - return -EINVAL; > - > - state = devm_kzalloc(&pdev->dev, sizeof(*state), GFP_KERNEL); > - if (!state) > - return -ENOMEM; > - > - state->gpios = devm_gpiod_get_array(&pdev->dev, "addr", GPIOD_OUT_LOW); > - if (IS_ERR(state->gpios)) > - return PTR_ERR(state->gpios); > - > - state->win_order = get_bitmask_order(resource_size(memory)) - 1; > - > - state->map.name = DRIVER_NAME; > - state->map.read = gf_read; > - state->map.copy_from = gf_copy_from; > - state->map.write = gf_write; > - state->map.copy_to = gf_copy_to; > - state->map.bankwidth = pdata->width; > - state->map.size = BIT(state->win_order + state->gpios->ndescs); > - state->map.virt = devm_ioremap_resource(&pdev->dev, memory); > - if (IS_ERR(state->map.virt)) > - return PTR_ERR(state->map.virt); > - > - state->map.phys = NO_XIP; > - state->map.map_priv_1 = (unsigned long)state; > - > - platform_set_drvdata(pdev, state); > - > - dev_notice(&pdev->dev, "probing %d-bit flash bus\n", > - state->map.bankwidth * 8); > - state->mtd = do_map_probe(memory->name, &state->map); > - if (!state->mtd) > - return -ENXIO; > - state->mtd->dev.parent = &pdev->dev; > - > - mtd_device_parse_register(state->mtd, part_probe_types, NULL, > - pdata->parts, pdata->nr_parts); > - > - return 0; > -} > - > -static int gpio_flash_remove(struct platform_device *pdev) > -{ > - struct async_state *state = platform_get_drvdata(pdev); > - > - mtd_device_unregister(state->mtd); > - map_destroy(state->mtd); > - return 0; > -} > - > -static struct platform_driver gpio_flash_driver = { > - .probe = gpio_flash_probe, > - .remove = gpio_flash_remove, > - .driver = { > - .name = DRIVER_NAME, > - }, > -}; > - > -module_platform_driver(gpio_flash_driver); > - > -MODULE_AUTHOR("Mike Frysinger <vapier@gentoo.org>"); > -MODULE_DESCRIPTION("MTD map driver for flashes addressed physically and with gpios"); > -MODULE_LICENSE("GPL"); > diff --git a/drivers/mtd/maps/physmap-core.c b/drivers/mtd/maps/physmap-core.c > index 7a50ff9ef812..2dc33ae71335 100644 > --- a/drivers/mtd/maps/physmap-core.c > +++ b/drivers/mtd/maps/physmap-core.c > @@ -13,6 +13,14 @@ > * > * Revised to handle newer style flash binding by: > * Copyright (C) 2007 David Gibson, IBM Corporation. > + * > + * GPIO address extension: > + * Handle the case where a flash device is mostly addressed using physical > + * line and supplemented by GPIOs. This way you can hook up say a 8MiB flash > + * to a 2MiB memory range and use the GPIOs to select a particular range. > + * > + * Copyright © 2000 Nicolas Pitre <nico@cam.org> > + * Copyright © 2005-2009 Analog Devices Inc. > */ > > #include <linux/module.h> > @@ -30,6 +38,7 @@ > #include <linux/mtd/cfi_endian.h> > #include <linux/io.h> > #include <linux/of_device.h> > +#include <linux/gpio/consumer.h> > > #include "physmap_of_gemini.h" > #include "physmap_of_versatile.h" > @@ -45,6 +54,9 @@ struct physmap_flash_info { > const char * const *part_types; > unsigned int nparts; > const struct mtd_partition *parts; > + struct gpio_descs *gpios; > + unsigned int gpio_values; > + unsigned int win_order; > }; > > static int physmap_flash_remove(struct platform_device *dev) > @@ -104,6 +116,119 @@ static void physmap_set_vpp(struct map_info *map, int state) > spin_unlock_irqrestore(&info->vpp_lock, flags); > } > > +#if IS_ENABLED(CONFIG_MTD_PHYSMAP_GPIO_ADDR) > +static void physmap_set_addr_gpios(struct physmap_flash_info *info, > + unsigned long ofs) > +{ > + unsigned int i; > + > + ofs >>= info->win_order; > + if (info->gpio_values == ofs) > + return; > + > + for (i = 0; i < info->gpios->ndescs; i++) { > + if ((BIT(i) & ofs) == (BIT(i) & info->gpio_values)) > + continue; > + > + gpiod_set_value(info->gpios->desc[i], !!(BIT(i) & ofs)); > + } > +} > + > +#define win_mask(order) (BIT(order) - 1) > + > +static map_word physmap_addr_gpios_read(struct map_info *map, > + unsigned long ofs) > +{ > + struct platform_device *pdev; > + struct physmap_flash_info *info; > + map_word mw; > + u16 word; > + > + pdev = (struct platform_device *)map->map_priv_1; > + info = platform_get_drvdata(pdev); > + physmap_set_addr_gpios(info, ofs); > + > + word = readw(map->virt + (ofs & win_mask(info->win_order))); > + mw.x[0] = word; > + return mw; > +} > + > +static void physmap_addr_gpios_copy_from(struct map_info *map, void *buf, > + unsigned long ofs, ssize_t len) > +{ > + struct platform_device *pdev; > + struct physmap_flash_info *info; > + > + pdev = (struct platform_device *)map->map_priv_1; > + info = platform_get_drvdata(pdev); > + > + while (len) { > + unsigned int winofs = ofs & win_mask(info->win_order); > + unsigned int chunklen = min_t(unsigned int, len, > + BIT(info->win_order) - winofs); > + > + physmap_set_addr_gpios(info, ofs); > + memcpy_fromio(buf, map->virt + winofs, chunklen); > + len -= chunklen; > + buf += chunklen; > + ofs += chunklen; > + } > +} > + > +static void physmap_addr_gpios_write(struct map_info *map, map_word mw, > + unsigned long ofs) > +{ > + struct platform_device *pdev; > + struct physmap_flash_info *info; > + u16 word; > + > + pdev = (struct platform_device *)map->map_priv_1; > + info = platform_get_drvdata(pdev); > + physmap_set_addr_gpios(info, ofs); > + > + word = mw.x[0]; > + writew(word, map->virt + (ofs & win_mask(info->win_order))); > +} > + > +static void physmap_addr_gpios_copy_to(struct map_info *map, unsigned long ofs, > + const void *buf, ssize_t len) > +{ > + struct platform_device *pdev; > + struct physmap_flash_info *info; > + > + pdev = (struct platform_device *)map->map_priv_1; > + info = platform_get_drvdata(pdev); > + > + while (len) { > + unsigned int winofs = ofs & win_mask(info->win_order); > + unsigned int chunklen = min_t(unsigned int, len, > + BIT(info->win_order) - winofs); > + > + physmap_set_addr_gpios(info, ofs); > + memcpy_toio(map->virt + winofs, buf, chunklen); > + len -= chunklen; > + buf += chunklen; > + ofs += chunklen; > + } > +} > + > +static int physmap_addr_gpios_map_init(struct map_info *map) > +{ > + map->phys = NO_XIP; > + map->read = physmap_addr_gpios_read; > + map->copy_from = physmap_addr_gpios_copy_from; > + map->write = physmap_addr_gpios_write; > + map->copy_to = physmap_addr_gpios_copy_to; > + > + return 0; > +} > +#else > +static int physmap_addr_gpios_map_init(struct map_info *map) > +{ > + return -ENOTSUPP; > +} > +#endif > + > #if IS_ENABLED(CONFIG_MTD_PHYSMAP_OF) > static const struct of_device_id of_flash_match[] = { > { > @@ -343,6 +468,16 @@ static int physmap_flash_probe(struct platform_device *dev) > > platform_set_drvdata(dev, info); > > + info->gpios = devm_gpiod_get_array_optional(&dev->dev, "addr", > + GPIOD_OUT_LOW); > + if (IS_ERR(info->gpios)) > + return PTR_ERR(info->gpios); > + > + if (info->gpios && info->nmaps > 1) { > + dev_err(&dev->dev, "addr-gpios only supported for nmaps == 1\n"); > + return -EINVAL; > + } > + > err = physmap_flash_of_init(dev); > if (err) > err = physmap_flash_pdata_init(dev); > @@ -368,10 +503,20 @@ static int physmap_flash_probe(struct platform_device *dev) > if (!info->maps[i].phys) > info->maps[i].phys = res->start; > > - info->maps[i].size = resource_size(res); > + info->win_order = get_bitmask_order(resource_size(res)) - 1; > + info->maps[i].size = BIT(info->win_order + > + (info->gpios ? > + info->gpios->ndescs : 0)); > + > info->maps[i].map_priv_1 = (unsigned long)dev; > > - simple_map_init(&info->maps[i]); > + if (info->gpios) { > + err = physmap_addr_gpios_map_init(&info->maps[i]); > + if (err) > + goto err_out; > + } else { > + simple_map_init(&info->maps[i]); > + } > > probe_type = rom_probe_types; > if (!info->probe_type) { > @@ -496,6 +641,7 @@ module_exit(physmap_exit); > MODULE_LICENSE("GPL"); > MODULE_AUTHOR("David Woodhouse <dwmw2@infradead.org>"); > MODULE_AUTHOR("Vitaly Wool <vwool@ru.mvista.com>"); > +MODULE_AUTHOR("Mike Frysinger <vapier@gentoo.org>"); > MODULE_DESCRIPTION("Generic configurable MTD map driver"); > > /* legacy platform drivers can't hotplug or coldplg */ > -- > 2.14.1 >
Hi Ricardo, On Tue, 9 Oct 2018 08:58:40 +0200 Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> wrote: > > + > > +static int physmap_flash_probe(struct platform_device *dev) > > +{ > > struct physmap_flash_info *info; > > const char * const *probe_type; > > - const char * const *part_types; > > int err = 0; > > int i; > > > > - physmap_data = dev_get_platdata(&dev->dev); > > - if (!physmap_data) > > - return -ENODEV; > > + if (!dev->dev.of_node && dev_get_platdata(&dev->dev)) > > + return -EINVAL; > > Maybe you want: > if (!dev->dev.of_node && !dev_get_platdata(&dev->dev)) Yes, I'll fix that. > > > > info = devm_kzalloc(&dev->dev, sizeof(*info), GFP_KERNEL); > > if (!info) > > @@ -132,14 +341,15 @@ static int physmap_flash_probe(struct platform_device *dev) > > if (!info->mtds) > > return -ENOMEM; > > > > - if (physmap_data->init) { > > - err = physmap_data->init(dev); > > - if (err) > > - goto err_out; > > - } > > - > > platform_set_drvdata(dev, info); > > > > + err = physmap_flash_of_init(dev); > > + if (err) > > + err = physmap_flash_pdata_init(dev); > > + > > Maybe replace this with: > if (dev->dev.of_node) > err = physmap_flash_of_init(dev); > else > err = physmap_flash_pdata_init(dev); > > Otherwise we are hiding the returncode from of_init, which might be > defered probing. Will fix that one too. Thanks for your review. Boris
On Tue, 9 Oct 2018 09:04:51 +0200 Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> wrote: > Hi Boris > > Maybe we want to leave the pdata example. > > /** > * The platform resource layout expected looks something like: > * struct mtd_partition partitions[] = { ... }; > * struct physmap_flash_data flash_data = > ..... Sure, I'll add it back. > > > On Mon, Oct 8, 2018 at 10:10 PM Boris Brezillon > <boris.brezillon@bootlin.com> wrote: > > > > Controlling some MSB address lines using GPIOs is just a small > > deviation of the generic physmap logic, and merging those two drivers > > allows us to share most of the probe logic, which is a good thing. > > > > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> > > --- > > drivers/mtd/maps/Kconfig | 19 ++- > > drivers/mtd/maps/Makefile | 1 - > > drivers/mtd/maps/gpio-addr-flash.c | 281 ------------------------------------- > > drivers/mtd/maps/physmap-core.c | 150 +++++++++++++++++++- > > 4 files changed, 157 insertions(+), 294 deletions(-) > > delete mode 100644 drivers/mtd/maps/gpio-addr-flash.c > > > > diff --git a/drivers/mtd/maps/Kconfig b/drivers/mtd/maps/Kconfig > > index 5bffebacce86..4fd13d76b7b5 100644 > > --- a/drivers/mtd/maps/Kconfig > > +++ b/drivers/mtd/maps/Kconfig > > @@ -94,6 +94,15 @@ config MTD_PHYSMAP_OF_GEMINI > > platforms, some detection and setting up parallel mode on the > > external interface. > > > > +config MTD_PHYSMAP_GPIO_ADDR > > + bool "GPIO-assisted Flash Chip Support" > > + depends on MTD_PHYSMAP > > + depends on GPIOLIB || COMPILE_TEST > > + depends on MTD_COMPLEX_MAPPINGS > > + help > > + Extend the physmap driver to allow flashes to be partially > > + physically addressed and assisted by GPIOs. > > + > > config MTD_PMC_MSP_EVM > > tristate "CFI Flash device mapped on PMC-Sierra MSP" > > depends on PMC_MSP && MTD_CFI > > @@ -334,16 +343,6 @@ config MTD_PCMCIA_ANONYMOUS > > > > If unsure, say N. > > > > -config MTD_GPIO_ADDR > > - tristate "GPIO-assisted Flash Chip Support" > > - depends on GPIOLIB || COMPILE_TEST > > - depends on MTD_COMPLEX_MAPPINGS > > - help > > - Map driver which allows flashes to be partially physically addressed > > - and assisted by GPIOs. > > - > > - If compiled as a module, it will be called gpio-addr-flash. > > - > > config MTD_UCLINUX > > bool "Generic uClinux RAM/ROM filesystem support" > > depends on (MTD_RAM=y || MTD_ROM=y) && (!MMU || COLDFIRE) > > diff --git a/drivers/mtd/maps/Makefile b/drivers/mtd/maps/Makefile > > index ad32b185a120..acec0fbfa18d 100644 > > --- a/drivers/mtd/maps/Makefile > > +++ b/drivers/mtd/maps/Makefile > > @@ -43,6 +43,5 @@ obj-$(CONFIG_MTD_PLATRAM) += plat-ram.o > > obj-$(CONFIG_MTD_INTEL_VR_NOR) += intel_vr_nor.o > > obj-$(CONFIG_MTD_RBTX4939) += rbtx4939-flash.o > > obj-$(CONFIG_MTD_VMU) += vmu-flash.o > > -obj-$(CONFIG_MTD_GPIO_ADDR) += gpio-addr-flash.o > > obj-$(CONFIG_MTD_LATCH_ADDR) += latch-addr-flash.o > > obj-$(CONFIG_MTD_LANTIQ) += lantiq-flash.o > > diff --git a/drivers/mtd/maps/gpio-addr-flash.c b/drivers/mtd/maps/gpio-addr-flash.c > > deleted file mode 100644 > > index a20e85aa770e..000000000000 > > --- a/drivers/mtd/maps/gpio-addr-flash.c > > +++ /dev/null > > @@ -1,281 +0,0 @@ > > -/* > > - * drivers/mtd/maps/gpio-addr-flash.c > > - * > > - * Handle the case where a flash device is mostly addressed using physical > > - * line and supplemented by GPIOs. This way you can hook up say a 8MiB flash > > - * to a 2MiB memory range and use the GPIOs to select a particular range. > > - * > > - * Copyright © 2000 Nicolas Pitre <nico@cam.org> > > - * Copyright © 2005-2009 Analog Devices Inc. > > - * > > - * Enter bugs at http://blackfin.uclinux.org/ > > - * > > - * Licensed under the GPL-2 or later. > > - */ > > - > > -#include <linux/gpio.h> > > -#include <linux/gpio/consumer.h> > > -#include <linux/io.h> > > -#include <linux/kernel.h> > > -#include <linux/module.h> > > -#include <linux/mtd/mtd.h> > > -#include <linux/mtd/map.h> > > -#include <linux/mtd/partitions.h> > > -#include <linux/mtd/physmap.h> > > -#include <linux/platform_device.h> > > -#include <linux/slab.h> > > -#include <linux/types.h> > > - > > -#define win_mask(x) ((BIT(x)) - 1) > > - > > -#define DRIVER_NAME "gpio-addr-flash" > > - > > -/** > > - * struct async_state - keep GPIO flash state > > - * @mtd: MTD state for this mapping > > - * @map: MTD map state for this flash > > - * @gpios: Struct containing the array of GPIO descriptors > > - * @gpio_values: cached GPIO values > > - * @win_order: dedicated memory size (if no GPIOs) > > - */ > > -struct async_state { > > - struct mtd_info *mtd; > > - struct map_info map; > > - struct gpio_descs *gpios; > > - unsigned int gpio_values; > > - unsigned int win_order; > > -}; > > -#define gf_map_info_to_state(mi) ((struct async_state *)(mi)->map_priv_1) > > - > > -/** > > - * gf_set_gpios() - set GPIO address lines to access specified flash offset > > - * @state: GPIO flash state > > - * @ofs: desired offset to access > > - * > > - * Rather than call the GPIO framework every time, cache the last-programmed > > - * value. This speeds up sequential accesses (which are by far the most common > > - * type). > > - */ > > -static void gf_set_gpios(struct async_state *state, unsigned long ofs) > > -{ > > - int i; > > - > > - ofs >>= state->win_order; > > - > > - if (ofs == state->gpio_values) > > - return; > > - > > - for (i = 0; i < state->gpios->ndescs; i++) { > > - if ((ofs & BIT(i)) == (state->gpio_values & BIT(i))) > > - continue; > > - > > - gpiod_set_value(state->gpios->desc[i], !!(ofs & BIT(i))); > > - } > > - > > - state->gpio_values = ofs; > > -} > > - > > -/** > > - * gf_read() - read a word at the specified offset > > - * @map: MTD map state > > - * @ofs: desired offset to read > > - */ > > -static map_word gf_read(struct map_info *map, unsigned long ofs) > > -{ > > - struct async_state *state = gf_map_info_to_state(map); > > - uint16_t word; > > - map_word test; > > - > > - gf_set_gpios(state, ofs); > > - > > - word = readw(map->virt + (ofs & win_mask(state->win_order))); > > - test.x[0] = word; > > - return test; > > -} > > - > > -/** > > - * gf_copy_from() - copy a chunk of data from the flash > > - * @map: MTD map state > > - * @to: memory to copy to > > - * @from: flash offset to copy from > > - * @len: how much to copy > > - * > > - * The "from" region may straddle more than one window, so toggle the GPIOs for > > - * each window region before reading its data. > > - */ > > -static void gf_copy_from(struct map_info *map, void *to, unsigned long from, ssize_t len) > > -{ > > - struct async_state *state = gf_map_info_to_state(map); > > - > > - int this_len; > > - > > - while (len) { > > - this_len = from & win_mask(state->win_order); > > - this_len = BIT(state->win_order) - this_len; > > - this_len = min_t(int, len, this_len); > > - > > - gf_set_gpios(state, from); > > - memcpy_fromio(to, > > - map->virt + (from & win_mask(state->win_order)), > > - this_len); > > - len -= this_len; > > - from += this_len; > > - to += this_len; > > - } > > -} > > - > > -/** > > - * gf_write() - write a word at the specified offset > > - * @map: MTD map state > > - * @ofs: desired offset to write > > - */ > > -static void gf_write(struct map_info *map, map_word d1, unsigned long ofs) > > -{ > > - struct async_state *state = gf_map_info_to_state(map); > > - uint16_t d; > > - > > - gf_set_gpios(state, ofs); > > - > > - d = d1.x[0]; > > - writew(d, map->virt + (ofs & win_mask(state->win_order))); > > -} > > - > > -/** > > - * gf_copy_to() - copy a chunk of data to the flash > > - * @map: MTD map state > > - * @to: flash offset to copy to > > - * @from: memory to copy from > > - * @len: how much to copy > > - * > > - * See gf_copy_from() caveat. > > - */ > > -static void gf_copy_to(struct map_info *map, unsigned long to, > > - const void *from, ssize_t len) > > -{ > > - struct async_state *state = gf_map_info_to_state(map); > > - > > - int this_len; > > - > > - while (len) { > > - this_len = to & win_mask(state->win_order); > > - this_len = BIT(state->win_order) - this_len; > > - this_len = min_t(int, len, this_len); > > - > > - gf_set_gpios(state, to); > > - memcpy_toio(map->virt + (to & win_mask(state->win_order)), > > - from, len); > > - > > - len -= this_len; > > - to += this_len; > > - from += this_len; > > - } > > -} > > - > > -static const char * const part_probe_types[] = { > > - "cmdlinepart", "RedBoot", NULL }; > > - > > -/** > > - * gpio_flash_probe() - setup a mapping for a GPIO assisted flash > > - * @pdev: platform device > > - * > > - * The platform resource layout expected looks something like: > > - * struct mtd_partition partitions[] = { ... }; > > - * struct physmap_flash_data flash_data = { ... }; > > - * static struct gpiod_lookup_table addr_flash_gpios = { > > - * .dev_id = "gpio-addr-flash.0", > > - * .table = { > > - * GPIO_LOOKUP_IDX("gpio.0", 15, "addr", 0, GPIO_ACTIVE_HIGH), > > - * GPIO_LOOKUP_IDX("gpio.0", 16, "addr", 1, GPIO_ACTIVE_HIGH), > > - * ); > > - * }; > > - * gpiod_add_lookup_table(&addr_flash_gpios); > > - * > > - * struct resource flash_resource[] = { > > - * { > > - * .name = "cfi_probe", > > - * .start = 0x20000000, > > - * .end = 0x201fffff, > > - * .flags = IORESOURCE_MEM, > > - * }, > > - * }; > > - * struct platform_device flash_device = { > > - * .name = "gpio-addr-flash", > > - * .dev = { .platform_data = &flash_data, }, > > - * .num_resources = ARRAY_SIZE(flash_resource), > > - * .resource = flash_resource, > > - * ... > > - * }; > > - */ > > -static int gpio_flash_probe(struct platform_device *pdev) > > -{ > > - struct physmap_flash_data *pdata; > > - struct resource *memory; > > - struct async_state *state; > > - > > - pdata = dev_get_platdata(&pdev->dev); > > - memory = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > - > > - if (!memory) > > - return -EINVAL; > > - > > - state = devm_kzalloc(&pdev->dev, sizeof(*state), GFP_KERNEL); > > - if (!state) > > - return -ENOMEM; > > - > > - state->gpios = devm_gpiod_get_array(&pdev->dev, "addr", GPIOD_OUT_LOW); > > - if (IS_ERR(state->gpios)) > > - return PTR_ERR(state->gpios); > > - > > - state->win_order = get_bitmask_order(resource_size(memory)) - 1; > > - > > - state->map.name = DRIVER_NAME; > > - state->map.read = gf_read; > > - state->map.copy_from = gf_copy_from; > > - state->map.write = gf_write; > > - state->map.copy_to = gf_copy_to; > > - state->map.bankwidth = pdata->width; > > - state->map.size = BIT(state->win_order + state->gpios->ndescs); > > - state->map.virt = devm_ioremap_resource(&pdev->dev, memory); > > - if (IS_ERR(state->map.virt)) > > - return PTR_ERR(state->map.virt); > > - > > - state->map.phys = NO_XIP; > > - state->map.map_priv_1 = (unsigned long)state; > > - > > - platform_set_drvdata(pdev, state); > > - > > - dev_notice(&pdev->dev, "probing %d-bit flash bus\n", > > - state->map.bankwidth * 8); > > - state->mtd = do_map_probe(memory->name, &state->map); > > - if (!state->mtd) > > - return -ENXIO; > > - state->mtd->dev.parent = &pdev->dev; > > - > > - mtd_device_parse_register(state->mtd, part_probe_types, NULL, > > - pdata->parts, pdata->nr_parts); > > - > > - return 0; > > -} > > - > > -static int gpio_flash_remove(struct platform_device *pdev) > > -{ > > - struct async_state *state = platform_get_drvdata(pdev); > > - > > - mtd_device_unregister(state->mtd); > > - map_destroy(state->mtd); > > - return 0; > > -} > > - > > -static struct platform_driver gpio_flash_driver = { > > - .probe = gpio_flash_probe, > > - .remove = gpio_flash_remove, > > - .driver = { > > - .name = DRIVER_NAME, > > - }, > > -}; > > - > > -module_platform_driver(gpio_flash_driver); > > - > > -MODULE_AUTHOR("Mike Frysinger <vapier@gentoo.org>"); > > -MODULE_DESCRIPTION("MTD map driver for flashes addressed physically and with gpios"); > > -MODULE_LICENSE("GPL"); > > diff --git a/drivers/mtd/maps/physmap-core.c b/drivers/mtd/maps/physmap-core.c > > index 7a50ff9ef812..2dc33ae71335 100644 > > --- a/drivers/mtd/maps/physmap-core.c > > +++ b/drivers/mtd/maps/physmap-core.c > > @@ -13,6 +13,14 @@ > > * > > * Revised to handle newer style flash binding by: > > * Copyright (C) 2007 David Gibson, IBM Corporation. > > + * > > + * GPIO address extension: > > + * Handle the case where a flash device is mostly addressed using physical > > + * line and supplemented by GPIOs. This way you can hook up say a 8MiB flash > > + * to a 2MiB memory range and use the GPIOs to select a particular range. > > + * > > + * Copyright © 2000 Nicolas Pitre <nico@cam.org> > > + * Copyright © 2005-2009 Analog Devices Inc. > > */ > > > > #include <linux/module.h> > > @@ -30,6 +38,7 @@ > > #include <linux/mtd/cfi_endian.h> > > #include <linux/io.h> > > #include <linux/of_device.h> > > +#include <linux/gpio/consumer.h> > > > > #include "physmap_of_gemini.h" > > #include "physmap_of_versatile.h" > > @@ -45,6 +54,9 @@ struct physmap_flash_info { > > const char * const *part_types; > > unsigned int nparts; > > const struct mtd_partition *parts; > > + struct gpio_descs *gpios; > > + unsigned int gpio_values; > > + unsigned int win_order; > > }; > > > > static int physmap_flash_remove(struct platform_device *dev) > > @@ -104,6 +116,119 @@ static void physmap_set_vpp(struct map_info *map, int state) > > spin_unlock_irqrestore(&info->vpp_lock, flags); > > } > > > > +#if IS_ENABLED(CONFIG_MTD_PHYSMAP_GPIO_ADDR) > > +static void physmap_set_addr_gpios(struct physmap_flash_info *info, > > + unsigned long ofs) > > +{ > > + unsigned int i; > > + > > + ofs >>= info->win_order; > > + if (info->gpio_values == ofs) > > + return; > > + > > + for (i = 0; i < info->gpios->ndescs; i++) { > > + if ((BIT(i) & ofs) == (BIT(i) & info->gpio_values)) > > + continue; > > + > > + gpiod_set_value(info->gpios->desc[i], !!(BIT(i) & ofs)); > > + } > > +} > > + > > +#define win_mask(order) (BIT(order) - 1) > > + > > +static map_word physmap_addr_gpios_read(struct map_info *map, > > + unsigned long ofs) > > +{ > > + struct platform_device *pdev; > > + struct physmap_flash_info *info; > > + map_word mw; > > + u16 word; > > + > > + pdev = (struct platform_device *)map->map_priv_1; > > + info = platform_get_drvdata(pdev); > > + physmap_set_addr_gpios(info, ofs); > > + > > + word = readw(map->virt + (ofs & win_mask(info->win_order))); > > + mw.x[0] = word; > > + return mw; > > +} > > + > > +static void physmap_addr_gpios_copy_from(struct map_info *map, void *buf, > > + unsigned long ofs, ssize_t len) > > +{ > > + struct platform_device *pdev; > > + struct physmap_flash_info *info; > > + > > + pdev = (struct platform_device *)map->map_priv_1; > > + info = platform_get_drvdata(pdev); > > + > > + while (len) { > > + unsigned int winofs = ofs & win_mask(info->win_order); > > + unsigned int chunklen = min_t(unsigned int, len, > > + BIT(info->win_order) - winofs); > > + > > + physmap_set_addr_gpios(info, ofs); > > + memcpy_fromio(buf, map->virt + winofs, chunklen); > > + len -= chunklen; > > + buf += chunklen; > > + ofs += chunklen; > > + } > > +} > > + > > +static void physmap_addr_gpios_write(struct map_info *map, map_word mw, > > + unsigned long ofs) > > +{ > > + struct platform_device *pdev; > > + struct physmap_flash_info *info; > > + u16 word; > > + > > + pdev = (struct platform_device *)map->map_priv_1; > > + info = platform_get_drvdata(pdev); > > + physmap_set_addr_gpios(info, ofs); > > + > > + word = mw.x[0]; > > + writew(word, map->virt + (ofs & win_mask(info->win_order))); > > +} > > + > > +static void physmap_addr_gpios_copy_to(struct map_info *map, unsigned long ofs, > > + const void *buf, ssize_t len) > > +{ > > + struct platform_device *pdev; > > + struct physmap_flash_info *info; > > + > > + pdev = (struct platform_device *)map->map_priv_1; > > + info = platform_get_drvdata(pdev); > > + > > + while (len) { > > + unsigned int winofs = ofs & win_mask(info->win_order); > > + unsigned int chunklen = min_t(unsigned int, len, > > + BIT(info->win_order) - winofs); > > + > > + physmap_set_addr_gpios(info, ofs); > > + memcpy_toio(map->virt + winofs, buf, chunklen); > > + len -= chunklen; > > + buf += chunklen; > > + ofs += chunklen; > > + } > > +} > > + > > +static int physmap_addr_gpios_map_init(struct map_info *map) > > +{ > > + map->phys = NO_XIP; > > + map->read = physmap_addr_gpios_read; > > + map->copy_from = physmap_addr_gpios_copy_from; > > + map->write = physmap_addr_gpios_write; > > + map->copy_to = physmap_addr_gpios_copy_to; > > + > > + return 0; > > +} > > +#else > > +static int physmap_addr_gpios_map_init(struct map_info *map) > > +{ > > + return -ENOTSUPP; > > +} > > +#endif > > + > > #if IS_ENABLED(CONFIG_MTD_PHYSMAP_OF) > > static const struct of_device_id of_flash_match[] = { > > { > > @@ -343,6 +468,16 @@ static int physmap_flash_probe(struct platform_device *dev) > > > > platform_set_drvdata(dev, info); > > > > + info->gpios = devm_gpiod_get_array_optional(&dev->dev, "addr", > > + GPIOD_OUT_LOW); > > + if (IS_ERR(info->gpios)) > > + return PTR_ERR(info->gpios); > > + > > + if (info->gpios && info->nmaps > 1) { > > + dev_err(&dev->dev, "addr-gpios only supported for nmaps == 1\n"); > > + return -EINVAL; > > + } > > + > > err = physmap_flash_of_init(dev); > > if (err) > > err = physmap_flash_pdata_init(dev); > > @@ -368,10 +503,20 @@ static int physmap_flash_probe(struct platform_device *dev) > > if (!info->maps[i].phys) > > info->maps[i].phys = res->start; > > > > - info->maps[i].size = resource_size(res); > > + info->win_order = get_bitmask_order(resource_size(res)) - 1; > > + info->maps[i].size = BIT(info->win_order + > > + (info->gpios ? > > + info->gpios->ndescs : 0)); > > + > > info->maps[i].map_priv_1 = (unsigned long)dev; > > > > - simple_map_init(&info->maps[i]); > > + if (info->gpios) { > > + err = physmap_addr_gpios_map_init(&info->maps[i]); > > + if (err) > > + goto err_out; > > + } else { > > + simple_map_init(&info->maps[i]); > > + } > > > > probe_type = rom_probe_types; > > if (!info->probe_type) { > > @@ -496,6 +641,7 @@ module_exit(physmap_exit); > > MODULE_LICENSE("GPL"); > > MODULE_AUTHOR("David Woodhouse <dwmw2@infradead.org>"); > > MODULE_AUTHOR("Vitaly Wool <vwool@ru.mvista.com>"); > > +MODULE_AUTHOR("Mike Frysinger <vapier@gentoo.org>"); > > MODULE_DESCRIPTION("Generic configurable MTD map driver"); > > > > /* legacy platform drivers can't hotplug or coldplg */ > > -- > > 2.14.1 > > > >
Hi Boris On Mon, Oct 8, 2018 at 10:10 PM Boris Brezillon <boris.brezillon@bootlin.com> wrote: > > Stop manipulating the dev->resource array directly and use the > platform_get_resource() helper instead. > > While at it, fix the loop check so that we never overflow the info->maps > and info->mtds array even if the number of resources attached to the > platform dev is higher than MAX_RESOURCES. > > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> > --- > drivers/mtd/maps/physmap.c | 29 ++++++++++++++++++----------- > 1 file changed, 18 insertions(+), 11 deletions(-) > > diff --git a/drivers/mtd/maps/physmap.c b/drivers/mtd/maps/physmap.c > index 4010afee4a33..e5b15ec2cb04 100644 > --- a/drivers/mtd/maps/physmap.c > +++ b/drivers/mtd/maps/physmap.c > @@ -122,23 +122,28 @@ static int physmap_flash_probe(struct platform_device *dev) > > platform_set_drvdata(dev, info); > > - for (i = 0; i < dev->num_resources; i++) { > + for (i = 0; i < MAX_RESOURCES; i++) { > + struct resource *res; > + > + res = platform_get_resource(dev, IORESOURCE_MEM, i); > + if (res) > + break; Maybe if (!res) > + > printk(KERN_NOTICE "physmap platform flash device: %.8llx at %.8llx\n", > - (unsigned long long)resource_size(&dev->resource[i]), > - (unsigned long long)dev->resource[i].start); > + (unsigned long long)resource_size(res), > + (unsigned long long)res->start); > > - if (!devm_request_mem_region(&dev->dev, > - dev->resource[i].start, > - resource_size(&dev->resource[i]), > - dev_name(&dev->dev))) { > + if (!devm_request_mem_region(&dev->dev, res->start, > + resource_size(res), > + dev_name(&dev->dev))) { > dev_err(&dev->dev, "Could not reserve memory region\n"); > err = -ENOMEM; > goto err_out; > } > > info->maps[i].name = dev_name(&dev->dev); > - info->maps[i].phys = dev->resource[i].start; > - info->maps[i].size = resource_size(&dev->resource[i]); > + info->maps[i].phys = res->start; > + info->maps[i].size = resource_size(res); > info->maps[i].bankwidth = physmap_data->width; > info->maps[i].set_vpp = physmap_set_vpp; > info->maps[i].pfow_base = physmap_data->pfow_base; > @@ -172,9 +177,11 @@ static int physmap_flash_probe(struct platform_device *dev) > info->mtds[i]->dev.parent = &dev->dev; > } > > - if (devices_found == 1) { > + if (!devices_found) { > + err = -ENODEV; > + } else if (devices_found == 1) { > info->cmtd = info->mtds[0]; > - } else if (devices_found > 1) { > + } else { > /* > * We detected multiple devices. Concatenate them together. > */ > -- > 2.14.1 >
Hi Boris On Mon, Oct 8, 2018 at 10:10 PM Boris Brezillon <boris.brezillon@bootlin.com> wrote: > > Remove the MAX_RESOURCES limitation by dynamically allocating the > ->mtds[] and ->maps[] at probe time based on the number of iomem > resources attached to the platform device. > > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> > --- > drivers/mtd/maps/physmap.c | 44 +++++++++++++++++++++++++++----------------- > 1 file changed, 27 insertions(+), 17 deletions(-) > > diff --git a/drivers/mtd/maps/physmap.c b/drivers/mtd/maps/physmap.c > index cbc50b53ec9d..86679d149a49 100644 > --- a/drivers/mtd/maps/physmap.c > +++ b/drivers/mtd/maps/physmap.c > @@ -22,12 +22,11 @@ > #include <linux/mtd/concat.h> > #include <linux/io.h> > > -#define MAX_RESOURCES 4 > - > struct physmap_flash_info { > - struct mtd_info *mtds[MAX_RESOURCES]; > + unsigned int nmaps; > + struct mtd_info **mtds; > struct mtd_info *cmtd; > - struct map_info maps[MAX_RESOURCES]; > + struct map_info *maps; > spinlock_t vpp_lock; > int vpp_refcnt; > }; > @@ -50,7 +49,7 @@ static int physmap_flash_remove(struct platform_device *dev) > mtd_concat_destroy(info->cmtd); > } > > - for (i = 0; i < MAX_RESOURCES; i++) { > + for (i = 0; i < info->nmaps; i++) { > if (info->mtds[i] != NULL) > map_destroy(info->mtds[i]); > } > @@ -101,7 +100,6 @@ static int physmap_flash_probe(struct platform_device *dev) > const char * const *part_types; > int err = 0; > int i; > - int devices_found = 0; > > physmap_data = dev_get_platdata(&dev->dev); > if (physmap_data == NULL) > @@ -114,6 +112,24 @@ static int physmap_flash_probe(struct platform_device *dev) > goto err_out; > } > > + while (platform_get_resource(dev, IORESOURCE_MEM, info->nmaps)) > + info->nmaps++; Maybe you prefer: for (info->nmaps = 0; platform_get_resource(dev, IORESOURCE_MEM, info->nmaps) ; info->nmaps++); (your choice, they are probably compiled into the same assembly) > + > + if (!info->nmaps) > + return -ENODEV; > + > + info->maps = devm_kzalloc(&dev->dev, > + sizeof(*info->maps) * info->nmaps, > + GFP_KERNEL); > + if (!info->maps) > + return -ENOMEM; > + > + info->mtds = devm_kzalloc(&dev->dev, > + sizeof(*info->mtds) * info->nmaps, > + GFP_KERNEL); > + if (!info->mtds) > + return -ENOMEM; > + > if (physmap_data->init) { > err = physmap_data->init(dev); > if (err) > @@ -122,13 +138,10 @@ static int physmap_flash_probe(struct platform_device *dev) > > platform_set_drvdata(dev, info); > > - for (i = 0; i < MAX_RESOURCES; i++) { > + for (i = 0; i < info->nmaps; i++) { > struct resource *res; > > res = platform_get_resource(dev, IORESOURCE_MEM, i); > - if (res) > - break; > - > info->maps[i].virt = devm_ioremap_resource(&dev->dev, res); > if (IS_ERR(info->maps[i].virt)) { > err = PTR_ERR(info->maps[i].virt); > @@ -159,21 +172,18 @@ static int physmap_flash_probe(struct platform_device *dev) > dev_err(&dev->dev, "map_probe failed\n"); > err = -ENXIO; > goto err_out; > - } else { > - devices_found++; > } > info->mtds[i]->dev.parent = &dev->dev; > } > > - if (!devices_found) { > - err = -ENODEV; > - } else if (devices_found == 1) { > + if (info->nmaps == 1) { > info->cmtd = info->mtds[0]; > } else { > /* > * We detected multiple devices. Concatenate them together. > */ > - info->cmtd = mtd_concat_create(info->mtds, devices_found, dev_name(&dev->dev)); > + info->cmtd = mtd_concat_create(info->mtds, info->nmaps, > + dev_name(&dev->dev)); > if (info->cmtd == NULL) > err = -ENXIO; > } > @@ -199,7 +209,7 @@ static void physmap_flash_shutdown(struct platform_device *dev) > struct physmap_flash_info *info = platform_get_drvdata(dev); > int i; > > - for (i = 0; i < MAX_RESOURCES && info->mtds[i]; i++) > + for (i = 0; i < info->nmaps && info->mtds[i]; i++) > if (mtd_suspend(info->mtds[i]) == 0) > mtd_resume(info->mtds[i]); > } > -- > 2.14.1 >
Hi Boris On Mon, Oct 8, 2018 at 10:10 PM Boris Brezillon <boris.brezillon@bootlin.com> wrote: > > Fix the following coding style issues: > - != NULL and == NULL test replaced by ! (or nothing) > - split over 80 chars lines > - add missing braces in multi-line if() {} else {} statements > > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> > --- > drivers/mtd/maps/physmap.c | 33 ++++++++++++++++++--------------- > 1 file changed, 18 insertions(+), 15 deletions(-) > > diff --git a/drivers/mtd/maps/physmap.c b/drivers/mtd/maps/physmap.c > index 7d30f3524d35..e27051bc5dc6 100644 > --- a/drivers/mtd/maps/physmap.c > +++ b/drivers/mtd/maps/physmap.c > @@ -38,7 +38,7 @@ static int physmap_flash_remove(struct platform_device *dev) > int i, err; > > info = platform_get_drvdata(dev); > - if (info == NULL) > + if (!info) > return 0; > > physmap_data = dev_get_platdata(&dev->dev); > @@ -53,7 +53,7 @@ static int physmap_flash_remove(struct platform_device *dev) > } > > for (i = 0; i < info->nmaps; i++) { > - if (info->mtds[i] != NULL) > + if (info->mtds[i]) > map_destroy(info->mtds[i]); > } > > @@ -90,10 +90,12 @@ static void physmap_set_vpp(struct map_info *map, int state) > } > > static const char * const rom_probe_types[] = { > - "cfi_probe", "jedec_probe", "qinfo_probe", "map_rom", NULL }; > + "cfi_probe", "jedec_probe", "qinfo_probe", "map_rom", NULL > +}; > > static const char * const part_probe_types[] = { > - "cmdlinepart", "RedBoot", "afs", NULL }; > + "cmdlinepart", "RedBoot", "afs", NULL > +}; > > static int physmap_flash_probe(struct platform_device *dev) > { > @@ -105,11 +107,10 @@ static int physmap_flash_probe(struct platform_device *dev) > int i; > > physmap_data = dev_get_platdata(&dev->dev); > - if (physmap_data == NULL) > + if (!physmap_data) > return -ENODEV; > > - info = devm_kzalloc(&dev->dev, sizeof(struct physmap_flash_info), > - GFP_KERNEL); > + info = devm_kzalloc(&dev->dev, sizeof(*info), GFP_KERNEL); > if (!info) > return -ENOMEM; > > @@ -163,13 +164,16 @@ static int physmap_flash_probe(struct platform_device *dev) > simple_map_init(&info->maps[i]); > > probe_type = rom_probe_types; > - if (physmap_data->probe_type == NULL) { > - for (; info->mtds[i] == NULL && *probe_type != NULL; probe_type++) > - info->mtds[i] = do_map_probe(*probe_type, &info->maps[i]); > - } else > - info->mtds[i] = do_map_probe(physmap_data->probe_type, &info->maps[i]); > + if (!physmap_data->probe_type) { > + for (; !info->mtds[i] && *probe_type; probe_type++) > + info->mtds[i] = do_map_probe(*probe_type, > + &info->maps[i]); > + } else { > + info->mtds[i] = do_map_probe(physmap_data->probe_type, > + &info->maps[i]); > + } Now that you are at it, maybe you want to change the order of the if branches: if (true){ } else{ } and move: probe_type = rom_probe_types; into the branch and fix: const char * const *probe_type; If you do not want to change it, is also fine :P > > - if (info->mtds[i] == NULL) { > + if (!info->mtds[i]) { > dev_err(&dev->dev, "map_probe failed\n"); > err = -ENXIO; > goto err_out; > @@ -185,7 +189,7 @@ static int physmap_flash_probe(struct platform_device *dev) > */ > info->cmtd = mtd_concat_create(info->mtds, info->nmaps, > dev_name(&dev->dev)); > - if (info->cmtd == NULL) > + if (!info->cmtd) > err = -ENXIO; > } > if (err) > @@ -231,7 +235,6 @@ static struct platform_driver physmap_flash_driver = { > }, > }; > > - > #ifdef CONFIG_MTD_PHYSMAP_COMPAT > static struct physmap_flash_data physmap_flash_data = { > .width = CONFIG_MTD_PHYSMAP_BANKWIDTH, > -- > 2.14.1 >
On Tue, 9 Oct 2018 09:37:19 +0200 Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> wrote: > Hi Boris > On Mon, Oct 8, 2018 at 10:10 PM Boris Brezillon > <boris.brezillon@bootlin.com> wrote: > > > > Fix the following coding style issues: > > - != NULL and == NULL test replaced by ! (or nothing) > > - split over 80 chars lines > > - add missing braces in multi-line if() {} else {} statements > > > > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> > > --- > > drivers/mtd/maps/physmap.c | 33 ++++++++++++++++++--------------- > > 1 file changed, 18 insertions(+), 15 deletions(-) > > > > diff --git a/drivers/mtd/maps/physmap.c b/drivers/mtd/maps/physmap.c > > index 7d30f3524d35..e27051bc5dc6 100644 > > --- a/drivers/mtd/maps/physmap.c > > +++ b/drivers/mtd/maps/physmap.c > > @@ -38,7 +38,7 @@ static int physmap_flash_remove(struct platform_device *dev) > > int i, err; > > > > info = platform_get_drvdata(dev); > > - if (info == NULL) > > + if (!info) > > return 0; > > > > physmap_data = dev_get_platdata(&dev->dev); > > @@ -53,7 +53,7 @@ static int physmap_flash_remove(struct platform_device *dev) > > } > > > > for (i = 0; i < info->nmaps; i++) { > > - if (info->mtds[i] != NULL) > > + if (info->mtds[i]) > > map_destroy(info->mtds[i]); > > } > > > > @@ -90,10 +90,12 @@ static void physmap_set_vpp(struct map_info *map, int state) > > } > > > > static const char * const rom_probe_types[] = { > > - "cfi_probe", "jedec_probe", "qinfo_probe", "map_rom", NULL }; > > + "cfi_probe", "jedec_probe", "qinfo_probe", "map_rom", NULL > > +}; > > > > static const char * const part_probe_types[] = { > > - "cmdlinepart", "RedBoot", "afs", NULL }; > > + "cmdlinepart", "RedBoot", "afs", NULL > > +}; > > > > static int physmap_flash_probe(struct platform_device *dev) > > { > > @@ -105,11 +107,10 @@ static int physmap_flash_probe(struct platform_device *dev) > > int i; > > > > physmap_data = dev_get_platdata(&dev->dev); > > - if (physmap_data == NULL) > > + if (!physmap_data) > > return -ENODEV; > > > > - info = devm_kzalloc(&dev->dev, sizeof(struct physmap_flash_info), > > - GFP_KERNEL); > > + info = devm_kzalloc(&dev->dev, sizeof(*info), GFP_KERNEL); > > if (!info) > > return -ENOMEM; > > > > @@ -163,13 +164,16 @@ static int physmap_flash_probe(struct platform_device *dev) > > simple_map_init(&info->maps[i]); > > > > probe_type = rom_probe_types; > > - if (physmap_data->probe_type == NULL) { > > - for (; info->mtds[i] == NULL && *probe_type != NULL; probe_type++) > > - info->mtds[i] = do_map_probe(*probe_type, &info->maps[i]); > > - } else > > - info->mtds[i] = do_map_probe(physmap_data->probe_type, &info->maps[i]); > > + if (!physmap_data->probe_type) { > > + for (; !info->mtds[i] && *probe_type; probe_type++) > > + info->mtds[i] = do_map_probe(*probe_type, > > + &info->maps[i]); > > + } else { > > + info->mtds[i] = do_map_probe(physmap_data->probe_type, > > + &info->maps[i]); > > + } > > Now that you are at it, maybe you want to change the order of the if branches: > > if (true){ > } else{ > } > > and move: > probe_type = rom_probe_types; > into the branch > > and fix: > const char * const *probe_type; > > > If you do not want to change it, is also fine :P I don't mind changing that, but not in this patch :-). > > > > > - if (info->mtds[i] == NULL) { > > + if (!info->mtds[i]) { > > dev_err(&dev->dev, "map_probe failed\n"); > > err = -ENXIO; > > goto err_out; > > @@ -185,7 +189,7 @@ static int physmap_flash_probe(struct platform_device *dev) > > */ > > info->cmtd = mtd_concat_create(info->mtds, info->nmaps, > > dev_name(&dev->dev)); > > - if (info->cmtd == NULL) > > + if (!info->cmtd) > > err = -ENXIO; > > } > > if (err) > > @@ -231,7 +235,6 @@ static struct platform_driver physmap_flash_driver = { > > }, > > }; > > > > - > > #ifdef CONFIG_MTD_PHYSMAP_COMPAT > > static struct physmap_flash_data physmap_flash_data = { > > .width = CONFIG_MTD_PHYSMAP_BANKWIDTH, > > -- > > 2.14.1 > > > >
On Tue, 9 Oct 2018 09:31:44 +0200 Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> wrote: > Hi Boris > On Mon, Oct 8, 2018 at 10:10 PM Boris Brezillon > <boris.brezillon@bootlin.com> wrote: > > > > Remove the MAX_RESOURCES limitation by dynamically allocating the > > ->mtds[] and ->maps[] at probe time based on the number of iomem > > resources attached to the platform device. > > > > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> > > --- > > drivers/mtd/maps/physmap.c | 44 +++++++++++++++++++++++++++----------------- > > 1 file changed, 27 insertions(+), 17 deletions(-) > > > > diff --git a/drivers/mtd/maps/physmap.c b/drivers/mtd/maps/physmap.c > > index cbc50b53ec9d..86679d149a49 100644 > > --- a/drivers/mtd/maps/physmap.c > > +++ b/drivers/mtd/maps/physmap.c > > @@ -22,12 +22,11 @@ > > #include <linux/mtd/concat.h> > > #include <linux/io.h> > > > > -#define MAX_RESOURCES 4 > > - > > struct physmap_flash_info { > > - struct mtd_info *mtds[MAX_RESOURCES]; > > + unsigned int nmaps; > > + struct mtd_info **mtds; > > struct mtd_info *cmtd; > > - struct map_info maps[MAX_RESOURCES]; > > + struct map_info *maps; > > spinlock_t vpp_lock; > > int vpp_refcnt; > > }; > > @@ -50,7 +49,7 @@ static int physmap_flash_remove(struct platform_device *dev) > > mtd_concat_destroy(info->cmtd); > > } > > > > - for (i = 0; i < MAX_RESOURCES; i++) { > > + for (i = 0; i < info->nmaps; i++) { > > if (info->mtds[i] != NULL) > > map_destroy(info->mtds[i]); > > } > > @@ -101,7 +100,6 @@ static int physmap_flash_probe(struct platform_device *dev) > > const char * const *part_types; > > int err = 0; > > int i; > > - int devices_found = 0; > > > > physmap_data = dev_get_platdata(&dev->dev); > > if (physmap_data == NULL) > > @@ -114,6 +112,24 @@ static int physmap_flash_probe(struct platform_device *dev) > > goto err_out; > > } > > > > + while (platform_get_resource(dev, IORESOURCE_MEM, info->nmaps)) > > + info->nmaps++; > > Maybe you prefer: > for (info->nmaps = 0; platform_get_resource(dev, IORESOURCE_MEM, > info->nmaps) ; info->nmaps++); I don't like when while/for loop bodies are empty, that's why I went for the while() loop ;-). > > (your choice, they are probably compiled into the same assembly) > > > + > > + if (!info->nmaps) > > + return -ENODEV; > > + > > + info->maps = devm_kzalloc(&dev->dev, > > + sizeof(*info->maps) * info->nmaps, > > + GFP_KERNEL); > > + if (!info->maps) > > + return -ENOMEM; > > + > > + info->mtds = devm_kzalloc(&dev->dev, > > + sizeof(*info->mtds) * info->nmaps, > > + GFP_KERNEL); > > + if (!info->mtds) > > + return -ENOMEM; > > + > > if (physmap_data->init) { > > err = physmap_data->init(dev); > > if (err) > > @@ -122,13 +138,10 @@ static int physmap_flash_probe(struct platform_device *dev) > > > > platform_set_drvdata(dev, info); > > > > - for (i = 0; i < MAX_RESOURCES; i++) { > > + for (i = 0; i < info->nmaps; i++) { > > struct resource *res; > > > > res = platform_get_resource(dev, IORESOURCE_MEM, i); > > - if (res) > > - break; > > - > > info->maps[i].virt = devm_ioremap_resource(&dev->dev, res); > > if (IS_ERR(info->maps[i].virt)) { > > err = PTR_ERR(info->maps[i].virt); > > @@ -159,21 +172,18 @@ static int physmap_flash_probe(struct platform_device *dev) > > dev_err(&dev->dev, "map_probe failed\n"); > > err = -ENXIO; > > goto err_out; > > - } else { > > - devices_found++; > > } > > info->mtds[i]->dev.parent = &dev->dev; > > } > > > > - if (!devices_found) { > > - err = -ENODEV; > > - } else if (devices_found == 1) { > > + if (info->nmaps == 1) { > > info->cmtd = info->mtds[0]; > > } else { > > /* > > * We detected multiple devices. Concatenate them together. > > */ > > - info->cmtd = mtd_concat_create(info->mtds, devices_found, dev_name(&dev->dev)); > > + info->cmtd = mtd_concat_create(info->mtds, info->nmaps, > > + dev_name(&dev->dev)); > > if (info->cmtd == NULL) > > err = -ENXIO; > > } > > @@ -199,7 +209,7 @@ static void physmap_flash_shutdown(struct platform_device *dev) > > struct physmap_flash_info *info = platform_get_drvdata(dev); > > int i; > > > > - for (i = 0; i < MAX_RESOURCES && info->mtds[i]; i++) > > + for (i = 0; i < info->nmaps && info->mtds[i]; i++) > > if (mtd_suspend(info->mtds[i]) == 0) > > mtd_resume(info->mtds[i]); > > } > > -- > > 2.14.1 > > > >
On Tue, 9 Oct 2018 09:16:41 +0200 Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> wrote: > Hi Boris > On Mon, Oct 8, 2018 at 10:10 PM Boris Brezillon > <boris.brezillon@bootlin.com> wrote: > > > > Stop manipulating the dev->resource array directly and use the > > platform_get_resource() helper instead. > > > > While at it, fix the loop check so that we never overflow the info->maps > > and info->mtds array even if the number of resources attached to the > > platform dev is higher than MAX_RESOURCES. > > > > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> > > --- > > drivers/mtd/maps/physmap.c | 29 ++++++++++++++++++----------- > > 1 file changed, 18 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/mtd/maps/physmap.c b/drivers/mtd/maps/physmap.c > > index 4010afee4a33..e5b15ec2cb04 100644 > > --- a/drivers/mtd/maps/physmap.c > > +++ b/drivers/mtd/maps/physmap.c > > @@ -122,23 +122,28 @@ static int physmap_flash_probe(struct platform_device *dev) > > > > platform_set_drvdata(dev, info); > > > > - for (i = 0; i < dev->num_resources; i++) { > > + for (i = 0; i < MAX_RESOURCES; i++) { > > + struct resource *res; > > + > > + res = platform_get_resource(dev, IORESOURCE_MEM, i); > > + if (res) > > + break; > > Maybe if (!res) I'll fix that one.
On Tue, 9 Oct 2018 09:11:06 +0200 Boris Brezillon <boris.brezillon@bootlin.com> wrote: > On Tue, 9 Oct 2018 09:04:51 +0200 > Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> wrote: > > > Hi Boris > > > > Maybe we want to leave the pdata example. > > > > /** > > * The platform resource layout expected looks something like: > > * struct mtd_partition partitions[] = { ... }; > > * struct physmap_flash_data flash_data = > > ..... > > Sure, I'll add it back. Actually I changed my mind on that one. The physmap driver did not document that in the first place, so, if we want to add this example, we should do it in a separate patch, and maybe place it next to the physmap_flash_data struct definition. This being said, I don't see a lot of drivers documenting this sort of things and I doubt new platforms will use pdata to describe their device anyway.
On Tue, 9 Oct 2018 09:52:05 +0200 Boris Brezillon <boris.brezillon@bootlin.com> wrote: > > > @@ -163,13 +164,16 @@ static int physmap_flash_probe(struct platform_device *dev) > > > simple_map_init(&info->maps[i]); > > > > > > probe_type = rom_probe_types; > > > - if (physmap_data->probe_type == NULL) { > > > - for (; info->mtds[i] == NULL && *probe_type != NULL; probe_type++) > > > - info->mtds[i] = do_map_probe(*probe_type, &info->maps[i]); > > > - } else > > > - info->mtds[i] = do_map_probe(physmap_data->probe_type, &info->maps[i]); > > > + if (!physmap_data->probe_type) { > > > + for (; !info->mtds[i] && *probe_type; probe_type++) > > > + info->mtds[i] = do_map_probe(*probe_type, > > > + &info->maps[i]); > > > + } else { > > > + info->mtds[i] = do_map_probe(physmap_data->probe_type, > > > + &info->maps[i]); > > > + } > > > > Now that you are at it, maybe you want to change the order of the if branches: > > > > if (true){ > > } else{ > > } > > > > and move: > > probe_type = rom_probe_types; > > into the branch > > > > and fix: > > const char * const *probe_type; > > > > > > If you do not want to change it, is also fine :P > > I don't mind changing that, but not in this patch :-). Actually, I'll let you send a patch on top of my series if you still want to address that ;-).