Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754824Ab3C0W11 (ORCPT ); Wed, 27 Mar 2013 18:27:27 -0400 Received: from mail.linuxfoundation.org ([140.211.169.12]:37544 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753473Ab3C0W10 (ORCPT ); Wed, 27 Mar 2013 18:27:26 -0400 Date: Wed, 27 Mar 2013 15:27:23 -0700 From: Andrew Morton To: Philipp Zabel Cc: linux-kernel@vger.kernel.org, Arnd Bergmann , Greg Kroah-Hartman , Grant Likely , Rob Herring , Paul Gortmaker , Shawn Guo , Huang Shijie , Dong Aisheng , Matt Porter , Fabio Estevam , Javier Martin , kernel@pengutronix.de, devicetree-discuss@lists.ozlabs.org Subject: Re: [PATCH v9 RESEND 2/4] misc: Generic on-chip SRAM allocation driver Message-Id: <20130327152723.723cd65e884413df405abe2d@linux-foundation.org> In-Reply-To: <1363776767-2635-3-git-send-email-p.zabel@pengutronix.de> References: <1363776767-2635-1-git-send-email-p.zabel@pengutronix.de> <1363776767-2635-3-git-send-email-p.zabel@pengutronix.de> X-Mailer: Sylpheed 3.2.0beta5 (GTK+ 2.24.10; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5348 Lines: 160 On Wed, 20 Mar 2013 11:52:45 +0100 Philipp Zabel 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 Subject: misc-generic-on-chip-sram-allocation-driver-fix fix Kconfig text, make sram_init static Cc: Dong Aisheng Cc: Fabio Estevam Cc: Grant Likely Cc: Greg Kroah-Hartman Cc: Huang Shijie Cc: Javier Martin Cc: Matt Porter Cc: Michal Simek Cc: Paul Gortmaker Cc: Philipp Zabel Cc: Rob Herring Cc: Shawn Guo Signed-off-by: Andrew Morton --- 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 --- a/drivers/misc/sram.c~misc-generic-on-chip-sram-allocation-driver-fix +++ a/drivers/misc/sram.c @@ -113,7 +113,7 @@ static struct platform_driver sram_drive .remove = sram_remove, }; -int __init sram_init(void) +static int __init sram_init(void) { return platform_driver_register(&sram_driver); } _ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/