On Mon, Dec 7, 2015 at 6:08 AM, Marek Szyprowski
<m.szyprowski@xxxxxxxxxxx> wrote:
> This patch allows device drivers to use more than one reserved memory
> region assigned to given device. When NULL name is passed to
> of_reserved_mem_device_init(), the default (first) region is used.
Every property that's an array does not need a name property. Just use
indexes please.
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
> ---
> drivers/of/of_reserved_mem.c | 101 +++++++++++++++++++++++++++++++---------
> include/linux/of_reserved_mem.h | 6 ++-
> 2 files changed, 84 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> index 1a3556a9e9ea..0a0b23b73004 100644
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c
> @@ -21,6 +21,7 @@
> #include <linux/sizes.h>
> #include <linux/of_reserved_mem.h>
> #include <linux/sort.h>
> +#include <linux/slab.h>
>
> #define MAX_RESERVED_REGIONS 16
> static struct reserved_mem reserved_mem[MAX_RESERVED_REGIONS];
> @@ -287,31 +288,84 @@ static inline struct reserved_mem *__find_rmem(struct device_node *node)
> return NULL;
> }
>
> +static struct reserved_mem *__node_to_rmem(struct device_node *node,
> + const char *name)
> +{
> + struct reserved_mem *rmem;
> + struct device_node *target;
> + int idx = 0;
> +
> + if (!node)
> + return NULL;
> +
> + if (name) {
> + idx = of_property_match_string(node,
> + "memory-region-names", name);
> + if (idx < 0)
> + return NULL;
> + }
> +
> + target = of_parse_phandle(node, "memory-region", idx);
> + if (!target)
> + return NULL;
> + rmem = __find_rmem(target);
> + of_node_put(target);
> +
> + return rmem;
> +}
> +
> +struct rmem_assigned_device {
> + struct device *dev;
> + struct reserved_mem *rmem;
> + struct list_head list;
> +};
> +
> +static LIST_HEAD(of_rmem_assigned_device_list);
> +static DEFINE_MUTEX(of_rmem_assigned_device_mutex);
Not that this is a fast or contended path, but I think a spinlock
would be more appropriate here.
> +
> /**
> * of_reserved_mem_device_init() - assign reserved memory region to given device
> + * @dev: Pointer to the device to configure
> + * @np: Pointer to the device_node with 'reserved-memory' property
> + * @name: Optional name of the selected region (can be NULL)
> + *
> + * This function assigns respective DMA-mapping operations based on reserved
> + * memory regionspecified by 'memory-region' property in @np node, named @name
> + * to the @dev device. When NULL name is provided, the default (first) memory
> + * region is used. When driver needs to use more than one reserved memory
> + * region, it should allocate child devices and initialize regions by name for
> + * each of child device.
> *
> - * This function assign memory region pointed by "memory-region" device tree
> - * property to the given device.
> + * Returns error code or zero on success.
> */
> -int of_reserved_mem_device_init(struct device *dev)
> +int of_reserved_mem_device_init(struct device *dev, struct device_node *np,
> + const char *name)
> {
> + struct rmem_assigned_device *rd;
> struct reserved_mem *rmem;
> - struct device_node *np;
> int ret;
>
> - np = of_parse_phandle(dev->of_node, "memory-region", 0);
> - if (!np)
> - return -ENODEV;
> -
> - rmem = __find_rmem(np);
> - of_node_put(np);
> -
> + rmem = __node_to_rmem(np, name);
> if (!rmem || !rmem->ops || !rmem->ops->device_init)
> return -EINVAL;
>
> + rd = kmalloc(sizeof(struct rmem_assigned_device), GFP_KERNEL);
> + if (!rd)
> + return -ENOMEM;
> +
> ret = rmem->ops->device_init(rmem, dev);
> - if (ret == 0)
> + if (ret == 0) {
> + rd->dev = dev;
> + rd->rmem = rmem;
> +
> + mutex_lock(&of_rmem_assigned_device_mutex);
> + list_add(&rd->list, &of_rmem_assigned_device_list);
> + mutex_unlock(&of_rmem_assigned_device_mutex);
> +
> dev_info(dev, "assigned reserved memory node %s\n", rmem->name);
> + } else {
> + kfree(rd);
> + }
>
> return ret;
> }
> @@ -319,21 +373,26 @@ EXPORT_SYMBOL_GPL(of_reserved_mem_device_init);
>
> /**
> * of_reserved_mem_device_release() - release reserved memory device structures
> + * @dev: Pointer to the device to deconfigure
> *
> * This function releases structures allocated for memory region handling for
> * the given device.
> */
> void of_reserved_mem_device_release(struct device *dev)
> {
> - struct reserved_mem *rmem;
> - struct device_node *np;
> -
> - np = of_parse_phandle(dev->of_node, "memory-region", 0);
> - if (!np)
> - return;
> -
> - rmem = __find_rmem(np);
> - of_node_put(np);
> + struct rmem_assigned_device *rd;
> + struct reserved_mem *rmem = NULL;
> +
> + mutex_lock(&of_rmem_assigned_device_mutex);
> + list_for_each_entry(rd, &of_rmem_assigned_device_list, list) {
> + if (rd->dev == dev) {
> + rmem = rd->rmem;
> + list_del(&rd->list);
> + kfree(rd);
> + break;
Is this function supposed to be called multiple times to release each
region. That's not very obvious and which region it removes undefined
for the call as it is just the first entry it finds.
Either both functions should init/release all regions or a single
specified region. I suppose there could be reasons not to init all
regions, but would expect that is the exception.
Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
