From mboxrd@z Thu Jan 1 00:00:00 1970 From: Philipp Zabel Subject: Re: [PATCH v8 2/4] misc: Generic on-chip SRAM allocation driver Date: Mon, 11 Feb 2013 19:15:24 +0100 Message-ID: <1360606524.4954.46.camel@pizza.hi.pengutronix.de> References: <1359977538-5859-1-git-send-email-p.zabel@pengutronix.de> <1359977538-5859-3-git-send-email-p.zabel@pengutronix.de> <20130208201634.B94C83E2C02@localhost> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20130208201634.B94C83E2C02@localhost> Sender: linux-kernel-owner@vger.kernel.org To: Grant Likely Cc: linux-kernel@vger.kernel.org, Arnd Bergmann , Greg Kroah-Hartman , Rob Herring , Paul Gortmaker , Shawn Guo , Richard Zhao , Huang Shijie , Dong Aisheng , Matt Porter , Fabio Estevam , Javier Martin , kernel@pengutronix.de, devicetree-discuss@lists.ozlabs.org List-Id: devicetree@vger.kernel.org Hi Grant, Am Freitag, den 08.02.2013, 20:16 +0000 schrieb Grant Likely: > On Mon, 4 Feb 2013 12:32:16 +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. > > > > Signed-off-by: Philipp Zabel > > Reviewed-by: Shawn Guo > > --- > > Changes since v7: > > - Removed obsolete __devinit/__devexit/__devexit_p > > --- > > Documentation/devicetree/bindings/misc/sram.txt | 17 ++++ > > drivers/misc/Kconfig | 9 ++ > > drivers/misc/Makefile | 1 + > > drivers/misc/sram.c | 121 +++++++++++++++++++++++ > > 4 files changed, 148 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 0000000..b64136c > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/misc/sram.txt > > @@ -0,0 +1,17 @@ > > +Generic on-chip SRAM > > + > > +Simple IO memory regions to be managed by the genalloc API. > > + > > +Required properties: > > + > > +- compatible : sram > > I'm a little concerned that 'sram' is just too generic for a compatible > value and we may end up needing a blacklist of systems where the sram > device should not be driven with this driver. If you can think of > a more descriptive name here then I would use it. various SoC vendors call this (variations of) "on-chip" or "internal" SRAM/memory. "on-chip-sram" or "internal-sram" are still plenty generic, though. How about "mmio-sram", as opposed to an SRAM that needs more than the simple mmio region handled by this driver? An alternative would be to use the vendor specific names and grow a compatible list in the driver ("fsl,ocram", "ti,ocm", ...). > However, I'm not worried about it enough to nak it and the rest of the > series looks fine. > > Acked-by: Grant Likely Thank you. regards Philipp