Re: [PATCH v9 RESEND 2/4] misc: Generic on-chip SRAM allocation driver
On Wed, 27 Mar 2013 15:27:23 -0700, Andrew Morton a...@linux-foundation.org wrote: On Wed, 20 Mar 2013 11:52:45 +0100 Philipp Zabel p.za...@pengutronix.de wrote: This driver requests and remaps a memory region as configured in the device tree. It serves memory from this region via the genalloc API. It optionally enables the SRAM clock. Other drivers can retrieve the genalloc pool from a phandle pointing to this drivers' device node in the device tree. The allocation granularity is hard-coded to 32 bytes for now, to make the SRAM driver useful for the 6502 remoteproc driver. There is overhead for bigger SRAMs, where only a much coarser allocation granularity is needed: At 32 bytes minimum allocation size, a 256 KiB SRAM needs a 1 KiB bitmap to track allocations. Documentation/devicetree/bindings/misc/sram.txt | 16 +++ drivers/misc/Kconfig|9 ++ drivers/misc/Makefile |1 + drivers/misc/sram.c | 121 +++ drivers/misc/sram.c is a pretty generic-sounding thing. Is it really Linux's One True SRAM driver? How many different sorts of sram devices do we expect this can be used with? If I don't use DT? In other words, perhaps this should have a more specific and accurate name? Well, it is SRAM. Not a whole lot of variation there, and all the driver does is allow bits of it to be carved up so that multiple device drivers don't step on each other's toes. I think that is pretty generic. If some SRAM comes along that is sufficiently different that it requires a different driver, then I'm pretty confident saying it would be the oddball, not this driver, and so would need the more specific name. All the users at the moment are DT platforms. When a non-DT user comes along, it won't be a problem to add a non-DT binding. In the mean time I wouldn't borrow trouble about it. g. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v9 RESEND 2/4] misc: Generic on-chip SRAM allocation driver
Hi Andrew, 2013/3/27 Andrew Morton a...@linux-foundation.org: On Wed, 20 Mar 2013 11:52:45 +0100 Philipp Zabel p.za...@pengutronix.de wrote: This driver requests and remaps a memory region as configured in the device tree. It serves memory from this region via the genalloc API. It optionally enables the SRAM clock. Other drivers can retrieve the genalloc pool from a phandle pointing to this drivers' device node in the device tree. The allocation granularity is hard-coded to 32 bytes for now, to make the SRAM driver useful for the 6502 remoteproc driver. There is overhead for bigger SRAMs, where only a much coarser allocation granularity is needed: At 32 bytes minimum allocation size, a 256 KiB SRAM needs a 1 KiB bitmap to track allocations. Documentation/devicetree/bindings/misc/sram.txt | 16 +++ drivers/misc/Kconfig|9 ++ drivers/misc/Makefile |1 + drivers/misc/sram.c | 121 +++ drivers/misc/sram.c is a pretty generic-sounding thing. Is it really Linux's One True SRAM driver? How many different sorts of sram devices do we expect this can be used with? If I don't use DT? I want to use it for xilinx microblaze BRAM and zynq OCM and BRAMS connected to buses on Microblaze, PPC and ARM zynq. I have there just one small problem with OCM because we have a suspend code which is copied to it and execute from it and current implementation has no handling for it because all memory you get is not executable. I am not sure how to handle this properly. Currently I am calling gen_pool_alloc which returns virtual address, then gen_pool_virt_to_phys to get physical address and then __arm_ioremap with MT_DEVICE . This works but it looks ugly. And also this is not generic solution because it doesn't work on Microblaze. Is there any other nice way how to ask for executable memory? Thanks, Michal -- Michal Simek, Ing. (M.Eng) w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/ Maintainer of Linux kernel - Xilinx Zynq ARM architecture Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v9 RESEND 2/4] misc: Generic on-chip SRAM allocation driver
Hi Andrew, thanks for taking care of these patches. Am Mittwoch, den 27.03.2013, 15:27 -0700 schrieb Andrew Morton: On Wed, 20 Mar 2013 11:52:45 +0100 Philipp Zabel p.za...@pengutronix.de wrote: This driver requests and remaps a memory region as configured in the device tree. It serves memory from this region via the genalloc API. It optionally enables the SRAM clock. Other drivers can retrieve the genalloc pool from a phandle pointing to this drivers' device node in the device tree. The allocation granularity is hard-coded to 32 bytes for now, to make the SRAM driver useful for the 6502 remoteproc driver. There is overhead for bigger SRAMs, where only a much coarser allocation granularity is needed: At 32 bytes minimum allocation size, a 256 KiB SRAM needs a 1 KiB bitmap to track allocations. Documentation/devicetree/bindings/misc/sram.txt | 16 +++ drivers/misc/Kconfig|9 ++ drivers/misc/Makefile |1 + drivers/misc/sram.c | 121 +++ drivers/misc/sram.c is a pretty generic-sounding thing. Is it really Linux's One True SRAM driver? How many different sorts of sram devices do we expect this can be used with? If I don't use DT? It's just a driver for MMIO-accessible SRAM ranges that are provided to other drivers (or suspend or dvfs platform code) via a genalloc pool. This is for system use, as opposed to an SRAM mtd device exporting SRAM to userspace. It's intended to unify all current SRAM genalloc pools in use. DT support is completely optional, the SRAM driver just uses platform resources. One limitation is that currently the allocation granularity is not configurable, that should be added to the driver. In other words, perhaps this should have a more specific and accurate name? Grant already made me change the compatible string to mmio-sram. We could change the driver name to mmio_sram, too. --- a/drivers/misc/Kconfig +++ b/drivers/misc/Kconfig @@ -510,6 +510,15 @@ config LATTICE_ECP3_CONFIG If unsure, say N. ... +static int sram_probe(struct platform_device *pdev) +{ + void __iomem *virt_base; + struct sram_dev *sram; + struct resource *res; + unsigned long size; + int ret; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!res) + return -EINVAL; + + size = resource_size(res); + + virt_base = devm_request_and_ioremap(pdev-dev, res); + if (!virt_base) + return -EADDRNOTAVAIL; EADDRNOTAVAIL is a networking error. If your users see this error pop up on their console they'll start wiggling ethernet cables, wondering why that didn't fix it. I should probably switch to devm_ioremap_resource() instead, which returns -EBUSY or -ENOMEM, depending on whether the request_region or ioremap fails. + sram = devm_kzalloc(pdev-dev, sizeof(*sram), GFP_KERNEL); + if (!sram) + return -ENOMEM; + + sram-clk = devm_clk_get(pdev-dev, NULL); + if (IS_ERR(sram-clk)) + sram-clk = NULL; + else + clk_prepare_enable(sram-clk); + + sram-pool = devm_gen_pool_create(pdev-dev, ilog2(SRAM_GRANULARITY), -1); + if (!sram-pool) + return -ENOMEM; + + ret = gen_pool_add_virt(sram-pool, (unsigned long)virt_base, + res-start, size, -1); + if (ret 0) { + gen_pool_destroy(sram-pool); + return ret; + } + + platform_set_drvdata(pdev, sram); + + dev_dbg(pdev-dev, SRAM pool: %ld KiB @ 0x%p\n, size / 1024, virt_base); + + return 0; +} ... +int __init sram_init(void) +{ + return platform_driver_register(sram_driver); +} + +postcore_initcall(sram_init); Why is it postcore_initcall()? Good question. A few architectures initialize their SRAM in a core_initcall (davinci, sh, mmp), a few from init_machine (omap2) postcore_initcall is when Freescale initialized their IRAM for i.MX. I have not tried yet to use SRAM to execute code for bus frequency changes during wfi or suspend, I'm not sure how early the sram pool is really needed by everyone. regards Philipp Fixlets: From: Andrew Morton a...@linux-foundation.org Subject: misc-generic-on-chip-sram-allocation-driver-fix fix Kconfig text, make sram_init static Cc: Dong Aisheng dong.aish...@linaro.org Cc: Fabio Estevam fabio.este...@freescale.com Cc: Grant Likely grant.lik...@secretlab.ca Cc: Greg Kroah-Hartman gre...@linuxfoundation.org Cc: Huang Shijie shij...@gmail.com Cc: Javier Martin javier.mar...@vista-silicon.com Cc: Matt Porter mpor...@ti.com Cc: Michal Simek mon...@monstr.eu Cc: Paul Gortmaker paul.gortma...@windriver.com Cc: Philipp Zabel p.za...@pengutronix.de Cc: Rob Herring rob.herr...@calxeda.com Cc: Shawn Guo shawn@linaro.org Signed-off-by: Andrew Morton a...@linux-foundation.org
Re: [PATCH v9 RESEND 2/4] misc: Generic on-chip SRAM allocation driver
On Wed, 20 Mar 2013 11:52:45 +0100 Philipp Zabel p.za...@pengutronix.de wrote: This driver requests and remaps a memory region as configured in the device tree. It serves memory from this region via the genalloc API. It optionally enables the SRAM clock. Other drivers can retrieve the genalloc pool from a phandle pointing to this drivers' device node in the device tree. The allocation granularity is hard-coded to 32 bytes for now, to make the SRAM driver useful for the 6502 remoteproc driver. There is overhead for bigger SRAMs, where only a much coarser allocation granularity is needed: At 32 bytes minimum allocation size, a 256 KiB SRAM needs a 1 KiB bitmap to track allocations. Documentation/devicetree/bindings/misc/sram.txt | 16 +++ drivers/misc/Kconfig|9 ++ drivers/misc/Makefile |1 + drivers/misc/sram.c | 121 +++ drivers/misc/sram.c is a pretty generic-sounding thing. Is it really Linux's One True SRAM driver? How many different sorts of sram devices do we expect this can be used with? If I don't use DT? In other words, perhaps this should have a more specific and accurate name? --- a/drivers/misc/Kconfig +++ b/drivers/misc/Kconfig @@ -510,6 +510,15 @@ config LATTICE_ECP3_CONFIG If unsure, say N. ... +static int sram_probe(struct platform_device *pdev) +{ + void __iomem *virt_base; + struct sram_dev *sram; + struct resource *res; + unsigned long size; + int ret; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!res) + return -EINVAL; + + size = resource_size(res); + + virt_base = devm_request_and_ioremap(pdev-dev, res); + if (!virt_base) + return -EADDRNOTAVAIL; EADDRNOTAVAIL is a networking error. If your users see this error pop up on their console they'll start wiggling ethernet cables, wondering why that didn't fix it. + sram = devm_kzalloc(pdev-dev, sizeof(*sram), GFP_KERNEL); + if (!sram) + return -ENOMEM; + + sram-clk = devm_clk_get(pdev-dev, NULL); + if (IS_ERR(sram-clk)) + sram-clk = NULL; + else + clk_prepare_enable(sram-clk); + + sram-pool = devm_gen_pool_create(pdev-dev, ilog2(SRAM_GRANULARITY), -1); + if (!sram-pool) + return -ENOMEM; + + ret = gen_pool_add_virt(sram-pool, (unsigned long)virt_base, + res-start, size, -1); + if (ret 0) { + gen_pool_destroy(sram-pool); + return ret; + } + + platform_set_drvdata(pdev, sram); + + dev_dbg(pdev-dev, SRAM pool: %ld KiB @ 0x%p\n, size / 1024, virt_base); + + return 0; +} ... +int __init sram_init(void) +{ + return platform_driver_register(sram_driver); +} + +postcore_initcall(sram_init); Why is it postcore_initcall()? Fixlets: From: Andrew Morton a...@linux-foundation.org Subject: misc-generic-on-chip-sram-allocation-driver-fix fix Kconfig text, make sram_init static Cc: Dong Aisheng dong.aish...@linaro.org Cc: Fabio Estevam fabio.este...@freescale.com Cc: Grant Likely grant.lik...@secretlab.ca Cc: Greg Kroah-Hartman gre...@linuxfoundation.org Cc: Huang Shijie shij...@gmail.com Cc: Javier Martin javier.mar...@vista-silicon.com Cc: Matt Porter mpor...@ti.com Cc: Michal Simek mon...@monstr.eu Cc: Paul Gortmaker paul.gortma...@windriver.com Cc: Philipp Zabel p.za...@pengutronix.de Cc: Rob Herring rob.herr...@calxeda.com Cc: Shawn Guo shawn@linaro.org Signed-off-by: Andrew Morton a...@linux-foundation.org --- drivers/misc/Kconfig |6 +++--- drivers/misc/sram.c |2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff -puN Documentation/devicetree/bindings/misc/sram.txt~misc-generic-on-chip-sram-allocation-driver-fix Documentation/devicetree/bindings/misc/sram.txt diff -puN drivers/misc/Kconfig~misc-generic-on-chip-sram-allocation-driver-fix drivers/misc/Kconfig --- a/drivers/misc/Kconfig~misc-generic-on-chip-sram-allocation-driver-fix +++ a/drivers/misc/Kconfig @@ -523,9 +523,9 @@ config SRAM depends on HAS_IOMEM select GENERIC_ALLOCATOR help - This driver allows to declare a memory region to be managed - by the genalloc API. It is supposed to be used for small - on-chip SRAM areas found on many SoCs. + This driver allows you to declare a memory region to be managed by + the genalloc API. It is supposed to be used for small on-chip SRAM + areas found on many SoCs. source drivers/misc/c2port/Kconfig source drivers/misc/eeprom/Kconfig diff -puN drivers/misc/Makefile~misc-generic-on-chip-sram-allocation-driver-fix drivers/misc/Makefile diff -puN drivers/misc/sram.c~misc-generic-on-chip-sram-allocation-driver-fix drivers/misc/sram.c ---
Re: [PATCH v9 RESEND 2/4] misc: Generic on-chip SRAM allocation driver
2013/3/20 Philipp Zabel p.za...@pengutronix.de: This driver requests and remaps a memory region as configured in the device tree. It serves memory from this region via the genalloc API. It optionally enables the SRAM clock. Other drivers can retrieve the genalloc pool from a phandle pointing to this drivers' device node in the device tree. The allocation granularity is hard-coded to 32 bytes for now, to make the SRAM driver useful for the 6502 remoteproc driver. There is overhead for bigger SRAMs, where only a much coarser allocation granularity is needed: At 32 bytes minimum allocation size, a 256 KiB SRAM needs a 1 KiB bitmap to track allocations. Signed-off-by: Philipp Zabel p.za...@pengutronix.de Reviewed-by: Shawn Guo shawn@linaro.org Acked-by: Grant Likely grant.lik...@secretlab.ca --- Changes since v8: - Changed device tree compatible string to mmio-sram --- Documentation/devicetree/bindings/misc/sram.txt | 16 +++ drivers/misc/Kconfig|9 ++ drivers/misc/Makefile |1 + drivers/misc/sram.c | 121 +++ 4 files changed, 147 insertions(+) create mode 100644 Documentation/devicetree/bindings/misc/sram.txt create mode 100644 drivers/misc/sram.c diff --git a/Documentation/devicetree/bindings/misc/sram.txt b/Documentation/devicetree/bindings/misc/sram.txt new file mode 100644 index 000..4d0a00e --- /dev/null +++ b/Documentation/devicetree/bindings/misc/sram.txt @@ -0,0 +1,16 @@ +Generic on-chip SRAM + +Simple IO memory regions to be managed by the genalloc API. + +Required properties: + +- compatible : mmio-sram + +- reg : SRAM iomem address range + +Example: + +sram: sram@5c00 { + compatible = mmio-sram; + reg = 0x5c00 0x4; /* 256 KiB SRAM at address 0x5c00 */ +}; diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index e83fdfe..4878507 100644 --- a/drivers/misc/Kconfig +++ b/drivers/misc/Kconfig @@ -510,6 +510,15 @@ config LATTICE_ECP3_CONFIG If unsure, say N. +config SRAM + bool Generic on-chip SRAM driver + depends on HAS_IOMEM + select GENERIC_ALLOCATOR + help + This driver allows to declare a memory region to be managed + by the genalloc API. It is supposed to be used for small + on-chip SRAM areas found on many SoCs. + source drivers/misc/c2port/Kconfig source drivers/misc/eeprom/Kconfig source drivers/misc/cb710/Kconfig diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile index 35a1463..08e2007 100644 --- a/drivers/misc/Makefile +++ b/drivers/misc/Makefile @@ -52,3 +52,4 @@ obj-$(CONFIG_INTEL_MEI) += mei/ obj-$(CONFIG_MAX8997_MUIC) += max8997-muic.o obj-$(CONFIG_VMWARE_VMCI) += vmw_vmci/ obj-$(CONFIG_LATTICE_ECP3_CONFIG) += lattice-ecp3-config.o +obj-$(CONFIG_SRAM) += sram.o diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c new file mode 100644 index 000..7858c62 --- /dev/null +++ b/drivers/misc/sram.c @@ -0,0 +1,121 @@ +/* + * Generic on-chip SRAM allocation driver + * + * Copyright (C) 2012 Philipp Zabel, Pengutronix + * + * 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. + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, + * MA 02110-1301, USA. + */ + +#include linux/kernel.h +#include linux/init.h +#include linux/clk.h +#include linux/err.h +#include linux/io.h +#include linux/of.h +#include linux/platform_device.h +#include linux/slab.h +#include linux/spinlock.h +#include linux/genalloc.h + +#define SRAM_GRANULARITY 32 + +struct sram_dev { + struct gen_pool *pool; + struct clk *clk; +}; + +static int sram_probe(struct platform_device *pdev) +{ + void __iomem *virt_base; + struct sram_dev *sram; + struct resource *res; + unsigned long size; + int ret; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!res) + return -EINVAL; + + size = resource_size(res); + + virt_base = devm_request_and_ioremap(pdev-dev, res); + if (!virt_base) + return -EADDRNOTAVAIL; + + sram = devm_kzalloc(pdev-dev, sizeof(*sram), GFP_KERNEL); + if (!sram) + return
[PATCH v9 RESEND 2/4] misc: Generic on-chip SRAM allocation driver
This driver requests and remaps a memory region as configured in the device tree. It serves memory from this region via the genalloc API. It optionally enables the SRAM clock. Other drivers can retrieve the genalloc pool from a phandle pointing to this drivers' device node in the device tree. The allocation granularity is hard-coded to 32 bytes for now, to make the SRAM driver useful for the 6502 remoteproc driver. There is overhead for bigger SRAMs, where only a much coarser allocation granularity is needed: At 32 bytes minimum allocation size, a 256 KiB SRAM needs a 1 KiB bitmap to track allocations. Signed-off-by: Philipp Zabel p.za...@pengutronix.de Reviewed-by: Shawn Guo shawn@linaro.org Acked-by: Grant Likely grant.lik...@secretlab.ca --- Changes since v8: - Changed device tree compatible string to mmio-sram --- Documentation/devicetree/bindings/misc/sram.txt | 16 +++ drivers/misc/Kconfig|9 ++ drivers/misc/Makefile |1 + drivers/misc/sram.c | 121 +++ 4 files changed, 147 insertions(+) create mode 100644 Documentation/devicetree/bindings/misc/sram.txt create mode 100644 drivers/misc/sram.c diff --git a/Documentation/devicetree/bindings/misc/sram.txt b/Documentation/devicetree/bindings/misc/sram.txt new file mode 100644 index 000..4d0a00e --- /dev/null +++ b/Documentation/devicetree/bindings/misc/sram.txt @@ -0,0 +1,16 @@ +Generic on-chip SRAM + +Simple IO memory regions to be managed by the genalloc API. + +Required properties: + +- compatible : mmio-sram + +- reg : SRAM iomem address range + +Example: + +sram: sram@5c00 { + compatible = mmio-sram; + reg = 0x5c00 0x4; /* 256 KiB SRAM at address 0x5c00 */ +}; diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index e83fdfe..4878507 100644 --- a/drivers/misc/Kconfig +++ b/drivers/misc/Kconfig @@ -510,6 +510,15 @@ config LATTICE_ECP3_CONFIG If unsure, say N. +config SRAM + bool Generic on-chip SRAM driver + depends on HAS_IOMEM + select GENERIC_ALLOCATOR + help + This driver allows to declare a memory region to be managed + by the genalloc API. It is supposed to be used for small + on-chip SRAM areas found on many SoCs. + source drivers/misc/c2port/Kconfig source drivers/misc/eeprom/Kconfig source drivers/misc/cb710/Kconfig diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile index 35a1463..08e2007 100644 --- a/drivers/misc/Makefile +++ b/drivers/misc/Makefile @@ -52,3 +52,4 @@ obj-$(CONFIG_INTEL_MEI) += mei/ obj-$(CONFIG_MAX8997_MUIC) += max8997-muic.o obj-$(CONFIG_VMWARE_VMCI) += vmw_vmci/ obj-$(CONFIG_LATTICE_ECP3_CONFIG) += lattice-ecp3-config.o +obj-$(CONFIG_SRAM) += sram.o diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c new file mode 100644 index 000..7858c62 --- /dev/null +++ b/drivers/misc/sram.c @@ -0,0 +1,121 @@ +/* + * Generic on-chip SRAM allocation driver + * + * Copyright (C) 2012 Philipp Zabel, Pengutronix + * + * 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. + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, + * MA 02110-1301, USA. + */ + +#include linux/kernel.h +#include linux/init.h +#include linux/clk.h +#include linux/err.h +#include linux/io.h +#include linux/of.h +#include linux/platform_device.h +#include linux/slab.h +#include linux/spinlock.h +#include linux/genalloc.h + +#define SRAM_GRANULARITY 32 + +struct sram_dev { + struct gen_pool *pool; + struct clk *clk; +}; + +static int sram_probe(struct platform_device *pdev) +{ + void __iomem *virt_base; + struct sram_dev *sram; + struct resource *res; + unsigned long size; + int ret; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!res) + return -EINVAL; + + size = resource_size(res); + + virt_base = devm_request_and_ioremap(pdev-dev, res); + if (!virt_base) + return -EADDRNOTAVAIL; + + sram = devm_kzalloc(pdev-dev, sizeof(*sram), GFP_KERNEL); + if (!sram) + return -ENOMEM; + + sram-clk = devm_clk_get(pdev-dev, NULL); + if (IS_ERR(sram-clk)) + sram-clk = NULL; + else + clk_prepare_enable(sram-clk); + +