Re: [PATCH 3/7] of: reserved_mem: add support for named reserved mem nodes
Hello, On 2015-12-08 15:58, Rob Herring wrote: On Mon, Dec 7, 2015 at 6:08 AM, Marek Szyprowski 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. Okay, I will update the patch and add support for indices in the main implementation as well as a wrapper, which accepts "name" parameter. Signed-off-by: Marek Szyprowski --- 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 #include #include +#include #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. This is not meant to be called really often and for all kinds on initialization lists and structures I saw that mutexes are used instead of spinlocks. There is no intention to let this function to be called from atomic context. + /** * 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_
Re: [PATCH 3/7] of: reserved_mem: add support for named reserved mem nodes
On Mon, Dec 7, 2015 at 6:08 AM, Marek Szyprowski 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 > --- > 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 > #include > #include > +#include > > #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); > -
[PATCH 3/7] of: reserved_mem: add support for named reserved mem nodes
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. Signed-off-by: Marek Szyprowski --- 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 #include #include +#include #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); + /** * 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; + } + }