| Message ID | 1313377794-26721-2-git-send-email-leoy@marvell.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Monday 15 August 2011 11:09:52 Leo Yan wrote: > On mmp platform, there have two sram banks: > audio sram and internal sram. The audio sram is mainly for audio; > the internal sram is for video, wtm and power management. > So add the sram allocator using genalloc to manage them. > > Every sram bank will register its own platform device > info, after the sram allocator create the generic pool > for the sram bank, the user module can use the pool's > name to get the pool handler; then it can use the handler > to alloc/free memory with genalloc APIs. > > Signed-off-by: Leo Yan <leoy@marvell.com> > --- > arch/arm/Kconfig | 1 + > arch/arm/mach-mmp/Makefile | 2 +- > arch/arm/mach-mmp/include/mach/sram.h | 35 +++++++ > arch/arm/mach-mmp/sram.c | 168 +++++++++++++++++++++++++++++++++ > 4 files changed, 205 insertions(+), 1 deletions(-) > create mode 100644 arch/arm/mach-mmp/include/mach/sram.h > create mode 100644 arch/arm/mach-mmp/sram.c Some time ago, there was talk of merging the existing sram drivers and creating a common driver that is easy to hook into. What has happened with that? My feeling is that we should stop adding more drivers like this in the platform code but rather put an authoritative copy into arch/arm/mm/ or even the top-level mm/ directory and change over the existing drivers to hook into that one. Arnd
On 10:59 Mon 15 Aug , Arnd Bergmann wrote: > On Monday 15 August 2011 11:09:52 Leo Yan wrote: > > On mmp platform, there have two sram banks: > > audio sram and internal sram. The audio sram is mainly for audio; > > the internal sram is for video, wtm and power management. > > So add the sram allocator using genalloc to manage them. > > > > Every sram bank will register its own platform device > > info, after the sram allocator create the generic pool > > for the sram bank, the user module can use the pool's > > name to get the pool handler; then it can use the handler > > to alloc/free memory with genalloc APIs. > > > > Signed-off-by: Leo Yan <leoy@marvell.com> > > --- > > arch/arm/Kconfig | 1 + > > arch/arm/mach-mmp/Makefile | 2 +- > > arch/arm/mach-mmp/include/mach/sram.h | 35 +++++++ > > arch/arm/mach-mmp/sram.c | 168 +++++++++++++++++++++++++++++++++ > > 4 files changed, 205 insertions(+), 1 deletions(-) > > create mode 100644 arch/arm/mach-mmp/include/mach/sram.h > > create mode 100644 arch/arm/mach-mmp/sram.c > > Some time ago, there was talk of merging the existing sram drivers > and creating a common driver that is easy to hook into. > > What has happened with that? My feeling is that we should stop adding > more drivers like this in the platform code but rather put an > authoritative copy into arch/arm/mm/ or even the top-level mm/ directory > and change over the existing drivers to hook into that one. no need anymore I send patch to add the support of phys/virt to genalloc so now we just have to use it Best Regards, J.
On 11:09 Mon 15 Aug , Leo Yan wrote: > On mmp platform, there have two sram banks: > audio sram and internal sram. The audio sram is mainly for audio; > the internal sram is for video, wtm and power management. > So add the sram allocator using genalloc to manage them. > > Every sram bank will register its own platform device > info, after the sram allocator create the generic pool > for the sram bank, the user module can use the pool's > name to get the pool handler; then it can use the handler > to alloc/free memory with genalloc APIs. > > Signed-off-by: Leo Yan <leoy@marvell.com> > --- > arch/arm/Kconfig | 1 + > arch/arm/mach-mmp/Makefile | 2 +- > arch/arm/mach-mmp/include/mach/sram.h | 35 +++++++ > arch/arm/mach-mmp/sram.c | 168 +++++++++++++++++++++++++++++++++ > 4 files changed, 205 insertions(+), 1 deletions(-) > create mode 100644 arch/arm/mach-mmp/include/mach/sram.h > create mode 100644 arch/arm/mach-mmp/sram.c > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig > index 2c71a8f..c7545d1 100644 > --- a/arch/arm/Kconfig > +++ b/arch/arm/Kconfig > @@ -559,6 +559,7 @@ config ARCH_MMP > select TICK_ONESHOT > select PLAT_PXA > select SPARSE_IRQ > + select GENERIC_ALLOCATOR > help > Support for Marvell's PXA168/PXA910(MMP) and MMP2 processor line. > > diff --git a/arch/arm/mach-mmp/Makefile b/arch/arm/mach-mmp/Makefile > index b0ac942..169c674 100644 > --- a/arch/arm/mach-mmp/Makefile > +++ b/arch/arm/mach-mmp/Makefile > @@ -7,7 +7,7 @@ obj-y += common.o clock.o devices.o time.o > # SoC support > obj-$(CONFIG_CPU_PXA168) += pxa168.o irq-pxa168.o > obj-$(CONFIG_CPU_PXA910) += pxa910.o irq-pxa168.o > -obj-$(CONFIG_CPU_MMP2) += mmp2.o irq-mmp2.o > +obj-$(CONFIG_CPU_MMP2) += mmp2.o irq-mmp2.o sram.o > > # board support > obj-$(CONFIG_MACH_ASPENITE) += aspenite.o > diff --git a/arch/arm/mach-mmp/include/mach/sram.h b/arch/arm/mach-mmp/include/mach/sram.h > new file mode 100644 > index 0000000..239e0fc > --- /dev/null > +++ b/arch/arm/mach-mmp/include/mach/sram.h > @@ -0,0 +1,35 @@ > +/* > + * linux/arch/arm/mach-mmp/include/mach/sram.h > + * > + * SRAM Memory Management > + * > + * Copyright (c) 2011 Marvell Semiconductors Inc. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + */ > + > +#ifndef __ASM_ARCH_SRAM_H > +#define __ASM_ARCH_SRAM_H > + > +#include <linux/genalloc.h> > + > +/* ARBITRARY: SRAM allocations are multiples of this 2^N size */ > +#define SRAM_GRANULARITY 512 > + > +enum sram_type { > + MMP_SRAM_UNDEFINED = 0, > + MMP_ASRAM, > + MMP_ISRAM, > +}; > + > +struct sram_platdata { > + char *pool_name; > + int granularity; > +}; > + > +extern struct gen_pool *sram_get_gpool(char *pool_name); > + > +#endif /* __ASM_ARCH_SRAM_H */ > diff --git a/arch/arm/mach-mmp/sram.c b/arch/arm/mach-mmp/sram.c > new file mode 100644 > index 0000000..4304f95 > --- /dev/null > +++ b/arch/arm/mach-mmp/sram.c > @@ -0,0 +1,168 @@ > +/* > + * linux/arch/arm/mach-mmp/sram.c > + * > + * based on mach-davinci/sram.c - DaVinci simple SRAM allocator > + * > + * Copyright (c) 2011 Marvell Semiconductors Inc. > + * All Rights Reserved > + * > + * Add for mmp sram support - Leo Yan <leoy@marvell.com> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + */ > + > +#include <linux/module.h> > +#include <linux/init.h> > +#include <linux/platform_device.h> > +#include <linux/io.h> > +#include <linux/err.h> > +#include <linux/slab.h> > +#include <linux/genalloc.h> > + > +#include <mach/sram.h> > + > +struct sram_bank_info { > + char *pool_name; > + struct gen_pool *gpool; > + int granularity; > + > + phys_addr_t sram_phys; > + void __iomem *sram_virt; > + u32 sram_size; > + > + struct list_head node; > +}; > + > +static DEFINE_MUTEX(sram_lock); > +static LIST_HEAD(sram_bank_list); > + > +struct gen_pool *sram_get_gpool(char *pool_name) > +{ > + struct sram_bank_info *info = NULL; > + > + if (!pool_name) > + return NULL; > + > + mutex_lock(&sram_lock); > + > + list_for_each_entry(info, &sram_bank_list, node) > + if (!strcmp(pool_name, info->pool_name)) > + break; > + > + mutex_unlock(&sram_lock); > + > + if (&info->node == &sram_bank_list) > + return NULL; > + > + return info->gpool; > +} > +EXPORT_SYMBOL(sram_get_gpool); > + > +static int __devinit sram_probe(struct platform_device *pdev) > +{ > + struct sram_platdata *pdata = pdev->dev.platform_data; > + struct sram_bank_info *info; > + struct resource *res; > + int ret = 0; > + > + if (!pdata && !pdata->pool_name) > + return -ENODEV; > + > + info = kzalloc(sizeof(*info), GFP_KERNEL); > + if (!info) > + return -ENOMEM; > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (res == NULL) { > + dev_err(&pdev->dev, "no memory resource defined\n"); > + ret = -ENODEV; > + goto out; > + } > + > + if (!resource_size(res)) > + return 0; > + > + info->sram_phys = (phys_addr_t)res->start; > + info->sram_size = resource_size(res); > + info->sram_virt = ioremap(info->sram_phys, info->sram_size); > + info->pool_name = kstrdup(pdata->pool_name, GFP_KERNEL); > + info->granularity = pdata->granularity; if we want name pool this need to be managed at gen alloc level directly not here Best Regards, J.
On 08/15/2011 04:59 PM, Arnd Bergmann wrote: > On Monday 15 August 2011 11:09:52 Leo Yan wrote: >> On mmp platform, there have two sram banks: >> audio sram and internal sram. The audio sram is mainly for audio; >> the internal sram is for video, wtm and power management. >> So add the sram allocator using genalloc to manage them. >> >> Every sram bank will register its own platform device >> info, after the sram allocator create the generic pool >> for the sram bank, the user module can use the pool's >> name to get the pool handler; then it can use the handler >> to alloc/free memory with genalloc APIs. >> >> Signed-off-by: Leo Yan<leoy@marvell.com> >> --- >> arch/arm/Kconfig | 1 + >> arch/arm/mach-mmp/Makefile | 2 +- >> arch/arm/mach-mmp/include/mach/sram.h | 35 +++++++ >> arch/arm/mach-mmp/sram.c | 168 +++++++++++++++++++++++++++++++++ >> 4 files changed, 205 insertions(+), 1 deletions(-) >> create mode 100644 arch/arm/mach-mmp/include/mach/sram.h >> create mode 100644 arch/arm/mach-mmp/sram.c > > Some time ago, there was talk of merging the existing sram drivers > and creating a common driver that is easy to hook into. > > What has happened with that? My feeling is that we should stop adding > more drivers like this in the platform code but rather put an > authoritative copy into arch/arm/mm/ or even the top-level mm/ directory > and change over the existing drivers to hook into that one. > > Arnd I think this is the mail thread for merging sram drivers: Consolidate SRAM support; Here is the latest status for this topic: http://lists.arm.linux.org.uk/lurker/message/20110710.121939.129161bf.en.html For JC's genalloc patches has been merged, in genalloc lib there has maintained the mapping for phys/virt address; so now just need to create a gen pool, and use this pool handler to alloc/free buffer, get the phys address, etc. My patches has refined the code with the new genalloc APIs. So you can see now the sram management is pretty simple, just create the gen pool, then later can directly access genalloc APIs.
On Mon, Aug 15, 2011 at 5:09 PM, Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> wrote: > On 10:59 Mon 15 Aug , Arnd Bergmann wrote: >> On Monday 15 August 2011 11:09:52 Leo Yan wrote: >> > On mmp platform, there have two sram banks: >> > audio sram and internal sram. The audio sram is mainly for audio; >> > the internal sram is for video, wtm and power management. >> > So add the sram allocator using genalloc to manage them. >> > >> > Every sram bank will register its own platform device >> > info, after the sram allocator create the generic pool >> > for the sram bank, the user module can use the pool's >> > name to get the pool handler; then it can use the handler >> > to alloc/free memory with genalloc APIs. >> > >> > Signed-off-by: Leo Yan <leoy@marvell.com> >> > --- >> > arch/arm/Kconfig | 1 + >> > arch/arm/mach-mmp/Makefile | 2 +- >> > arch/arm/mach-mmp/include/mach/sram.h | 35 +++++++ >> > arch/arm/mach-mmp/sram.c | 168 +++++++++++++++++++++++++++++++++ >> > 4 files changed, 205 insertions(+), 1 deletions(-) >> > create mode 100644 arch/arm/mach-mmp/include/mach/sram.h >> > create mode 100644 arch/arm/mach-mmp/sram.c >> >> Some time ago, there was talk of merging the existing sram drivers >> and creating a common driver that is easy to hook into. >> >> What has happened with that? My feeling is that we should stop adding >> more drivers like this in the platform code but rather put an >> authoritative copy into arch/arm/mm/ or even the top-level mm/ directory >> and change over the existing drivers to hook into that one. > no need anymore I send patch to add the support of phys/virt to genalloc so > now we just have to use it > > Best Regards, > J. > Exactly, and Russel already gave up on original patch. genpool is already a common driver. Thanks Haojian
On Mon, 15 Aug 2011, Arnd Bergmann wrote: > Some time ago, there was talk of merging the existing sram drivers > and creating a common driver that is easy to hook into. Absolutely. > What has happened with that? My feeling is that we should stop adding > more drivers like this in the platform code but rather put an > authoritative copy into arch/arm/mm/ or even the top-level mm/ directory > and change over the existing drivers to hook into that one. IIRC, this was driven by Russell. Maybe this is a good time to revive discussion around it. Nicolas
On Mon, Aug 15, 2011 at 09:23:49AM -0400, Nicolas Pitre wrote: > On Mon, 15 Aug 2011, Arnd Bergmann wrote: > > > Some time ago, there was talk of merging the existing sram drivers > > and creating a common driver that is easy to hook into. > > Absolutely. > > > What has happened with that? My feeling is that we should stop adding > > more drivers like this in the platform code but rather put an > > authoritative copy into arch/arm/mm/ or even the top-level mm/ directory > > and change over the existing drivers to hook into that one. > > IIRC, this was driven by Russell. Maybe this is a good time to revive > discussion around it. As I understand it, Jean-Christophe's patches were essentially a replacement for my infrastructure changes, and it is Jean-Christophe's patches which went in. So, my work is obsolete. I'm not sure what else there is to be done here - if Jean-Christophe's genpool interfaces are not sufficient, maybe when the discussion surrounding my patches and Jean-Christophe's patches happened, maybe someone should've sorted out finishing off Jean-Christophe's patches to the same extent that mine have. Or maybe my patches should've been merged instead of Jean-Christophe's and Jean-Christophe's brought forward to plug into the core of my patches. Either way, I completely gave up with the idea because I saw no future for my patches, and now I'm no longer interested in it.
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 2c71a8f..c7545d1 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -559,6 +559,7 @@ config ARCH_MMP select TICK_ONESHOT select PLAT_PXA select SPARSE_IRQ + select GENERIC_ALLOCATOR help Support for Marvell's PXA168/PXA910(MMP) and MMP2 processor line. diff --git a/arch/arm/mach-mmp/Makefile b/arch/arm/mach-mmp/Makefile index b0ac942..169c674 100644 --- a/arch/arm/mach-mmp/Makefile +++ b/arch/arm/mach-mmp/Makefile @@ -7,7 +7,7 @@ obj-y += common.o clock.o devices.o time.o # SoC support obj-$(CONFIG_CPU_PXA168) += pxa168.o irq-pxa168.o obj-$(CONFIG_CPU_PXA910) += pxa910.o irq-pxa168.o -obj-$(CONFIG_CPU_MMP2) += mmp2.o irq-mmp2.o +obj-$(CONFIG_CPU_MMP2) += mmp2.o irq-mmp2.o sram.o # board support obj-$(CONFIG_MACH_ASPENITE) += aspenite.o diff --git a/arch/arm/mach-mmp/include/mach/sram.h b/arch/arm/mach-mmp/include/mach/sram.h new file mode 100644 index 0000000..239e0fc --- /dev/null +++ b/arch/arm/mach-mmp/include/mach/sram.h @@ -0,0 +1,35 @@ +/* + * linux/arch/arm/mach-mmp/include/mach/sram.h + * + * SRAM Memory Management + * + * Copyright (c) 2011 Marvell Semiconductors Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + */ + +#ifndef __ASM_ARCH_SRAM_H +#define __ASM_ARCH_SRAM_H + +#include <linux/genalloc.h> + +/* ARBITRARY: SRAM allocations are multiples of this 2^N size */ +#define SRAM_GRANULARITY 512 + +enum sram_type { + MMP_SRAM_UNDEFINED = 0, + MMP_ASRAM, + MMP_ISRAM, +}; + +struct sram_platdata { + char *pool_name; + int granularity; +}; + +extern struct gen_pool *sram_get_gpool(char *pool_name); + +#endif /* __ASM_ARCH_SRAM_H */ diff --git a/arch/arm/mach-mmp/sram.c b/arch/arm/mach-mmp/sram.c new file mode 100644 index 0000000..4304f95 --- /dev/null +++ b/arch/arm/mach-mmp/sram.c @@ -0,0 +1,168 @@ +/* + * linux/arch/arm/mach-mmp/sram.c + * + * based on mach-davinci/sram.c - DaVinci simple SRAM allocator + * + * Copyright (c) 2011 Marvell Semiconductors Inc. + * All Rights Reserved + * + * Add for mmp sram support - Leo Yan <leoy@marvell.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + */ + +#include <linux/module.h> +#include <linux/init.h> +#include <linux/platform_device.h> +#include <linux/io.h> +#include <linux/err.h> +#include <linux/slab.h> +#include <linux/genalloc.h> + +#include <mach/sram.h> + +struct sram_bank_info { + char *pool_name; + struct gen_pool *gpool; + int granularity; + + phys_addr_t sram_phys; + void __iomem *sram_virt; + u32 sram_size; + + struct list_head node; +}; + +static DEFINE_MUTEX(sram_lock); +static LIST_HEAD(sram_bank_list); + +struct gen_pool *sram_get_gpool(char *pool_name) +{ + struct sram_bank_info *info = NULL; + + if (!pool_name) + return NULL; + + mutex_lock(&sram_lock); + + list_for_each_entry(info, &sram_bank_list, node) + if (!strcmp(pool_name, info->pool_name)) + break; + + mutex_unlock(&sram_lock); + + if (&info->node == &sram_bank_list) + return NULL; + + return info->gpool; +} +EXPORT_SYMBOL(sram_get_gpool); + +static int __devinit sram_probe(struct platform_device *pdev) +{ + struct sram_platdata *pdata = pdev->dev.platform_data; + struct sram_bank_info *info; + struct resource *res; + int ret = 0; + + if (!pdata && !pdata->pool_name) + return -ENODEV; + + info = kzalloc(sizeof(*info), GFP_KERNEL); + if (!info) + return -ENOMEM; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (res == NULL) { + dev_err(&pdev->dev, "no memory resource defined\n"); + ret = -ENODEV; + goto out; + } + + if (!resource_size(res)) + return 0; + + info->sram_phys = (phys_addr_t)res->start; + info->sram_size = resource_size(res); + info->sram_virt = ioremap(info->sram_phys, info->sram_size); + info->pool_name = kstrdup(pdata->pool_name, GFP_KERNEL); + info->granularity = pdata->granularity; + + info->gpool = gen_pool_create(ilog2(info->granularity), -1); + if (!info->gpool) { + dev_err(&pdev->dev, "create pool failed\n"); + ret = -ENOMEM; + goto create_pool_err; + } + + ret = gen_pool_add_virt(info->gpool, (unsigned long)info->sram_virt, + info->sram_phys, info->sram_size, -1); + if (ret < 0) { + dev_err(&pdev->dev, "add new chunk failed\n"); + ret = -ENOMEM; + goto add_chunk_err; + } + + mutex_lock(&sram_lock); + list_add(&info->node, &sram_bank_list); + mutex_unlock(&sram_lock); + + platform_set_drvdata(pdev, info); + + dev_info(&pdev->dev, "initialized\n"); + return 0; + +add_chunk_err: + gen_pool_destroy(info->gpool); +create_pool_err: + iounmap(info->sram_virt); + kfree(info->pool_name); +out: + kfree(info); + return ret; +} + +static int __devexit sram_remove(struct platform_device *pdev) +{ + struct sram_bank_info *info; + + info = platform_get_drvdata(pdev); + if (info == NULL) + return -ENODEV; + + mutex_lock(&sram_lock); + list_del(&info->node); + mutex_unlock(&sram_lock); + + gen_pool_destroy(info->gpool); + iounmap(info->sram_virt); + kfree(info->pool_name); + kfree(info); + return 0; +} + +static const struct platform_device_id sram_id_table[] = { + { "asram", MMP_ASRAM }, + { "isram", MMP_ISRAM }, + { } +}; + +static struct platform_driver sram_driver = { + .probe = sram_probe, + .remove = sram_remove, + .driver = { + .name = "mmp-sram", + }, + .id_table = sram_id_table, +}; + +static int __init sram_init(void) +{ + return platform_driver_register(&sram_driver); +} +core_initcall(sram_init); + +MODULE_LICENSE("GPL");
On mmp platform, there have two sram banks: audio sram and internal sram. The audio sram is mainly for audio; the internal sram is for video, wtm and power management. So add the sram allocator using genalloc to manage them. Every sram bank will register its own platform device info, after the sram allocator create the generic pool for the sram bank, the user module can use the pool's name to get the pool handler; then it can use the handler to alloc/free memory with genalloc APIs. Signed-off-by: Leo Yan <leoy@marvell.com> --- arch/arm/Kconfig | 1 + arch/arm/mach-mmp/Makefile | 2 +- arch/arm/mach-mmp/include/mach/sram.h | 35 +++++++ arch/arm/mach-mmp/sram.c | 168 +++++++++++++++++++++++++++++++++ 4 files changed, 205 insertions(+), 1 deletions(-) create mode 100644 arch/arm/mach-mmp/include/mach/sram.h create mode 100644 arch/arm/mach-mmp/sram.c