From mboxrd@z Thu Jan 1 00:00:00 1970 From: Rob Herring Subject: Re: [PATCH v3 2/6] misc: sram: add ability to mark sram sections as reserved Date: Sun, 7 Jul 2013 14:46:36 -0500 Message-ID: References: <201307051440.19428.heiko@sntech.de> <201307051441.27605.heiko@sntech.de> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: <201307051441.27605.heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: devicetree-discuss-bounces+gldd-devicetree-discuss=m.gmane.org-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org Sender: "devicetree-discuss" To: =?ISO-8859-1?Q?Heiko_St=FCbner?= Cc: Russell King , Greg Kroah-Hartman , "devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org" , "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , Grant Likely , Ulrich Prinz , Philipp Zabel , "linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org" List-Id: devicetree@vger.kernel.org On Fri, Jul 5, 2013 at 7:41 AM, Heiko St=FCbner wrote: > Some SoCs need parts of their sram for special purposes. So while being p= art > of the periphal, it should not be part of the genpool controlling the sra= m. s/periphal/peripheral/ > > Threfore add an option mmio-sram-reserved to keep arbitary portions of the s/Threfore/Therefore/ s/arbitary/arbitrary/ > sram from being part of the pool. > > Suggested-by: Rob Herring > Signed-off-by: Heiko Stuebner > Tested-by: Ulrich Prinz > --- > Documentation/devicetree/bindings/misc/sram.txt | 8 +++ > drivers/misc/sram.c | 80 +++++++++++++++++= ++++-- > 2 files changed, 82 insertions(+), 6 deletions(-) > > diff --git a/Documentation/devicetree/bindings/misc/sram.txt b/Documentat= ion/devicetree/bindings/misc/sram.txt > index 4d0a00e..eae080e 100644 > --- a/Documentation/devicetree/bindings/misc/sram.txt > +++ b/Documentation/devicetree/bindings/misc/sram.txt > @@ -8,9 +8,17 @@ Required properties: > > - reg : SRAM iomem address range > > +Optional properties: > + > +- mmio-sram-reserved: ordered list of reserved chunks inside the sram th= at > + should not become part of the genalloc pool. genalloc is linux specific. "should not be used by the OS" would be better. > + Format is , , ...; with base being relative to t= he > + reg property base. > + > Example: > > sram: sram@5c000000 { > compatible =3D "mmio-sram"; > reg =3D <0x5c000000 0x40000>; /* 256 KiB SRAM at address 0x5c0000= 00 */ > + mmio-sram-reserved =3D <0x0 0x100>; /* reserve 0x5c000000-0x5c000= 100 */ > }; > diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c > index afe66571..9131e4a 100644 > --- a/drivers/misc/sram.c > +++ b/drivers/misc/sram.c > @@ -42,6 +42,13 @@ static int sram_probe(struct platform_device *pdev) > struct sram_dev *sram; > struct resource *res; > unsigned long size; > + const __be32 *reserved_list =3D NULL; > + int reserved_size =3D 0; > + unsigned int cur_start =3D 0; > + unsigned int cur_size; > + unsigned int rstart; > + unsigned int rsize; Use one line for each type. > + int i; > int ret; > > res =3D platform_get_resource(pdev, IORESOURCE_MEM, 0); > @@ -65,12 +72,73 @@ static int sram_probe(struct platform_device *pdev) > if (!sram->pool) > return -ENOMEM; > > - ret =3D gen_pool_add_virt(sram->pool, (unsigned long)virt_base, > - res->start, size, -1); > - if (ret < 0) { > - if (sram->clk) > - clk_disable_unprepare(sram->clk); > - return ret; > + if (pdev->dev.of_node) { > + reserved_list =3D of_get_property(pdev->dev.of_node, > + "mmio-sram-reserved", > + &reserved_size); > + if (reserved_list) { > + reserved_size /=3D sizeof(*reserved_list); > + if (!reserved_size || reserved_size % 2) { > + dev_warn(&pdev->dev, "wrong number of arg= uments in mmio-sram-reserved\n"); > + reserved_list =3D NULL; > + } > + } > + } > + > + if (!reserved_list) > + reserved_size =3D 0; This can be moved to where you set reserved_list to NULL. > + > + for (i =3D 0; i < (reserved_size + 2); i +=3D 2) { > + if (i < reserved_size) { This loop is hard to follow. For example, it is not immediate obvious why you loop an extra time. I think it would be more straight forward to determine the free regions first and then do the allocations. This will require a kmalloc or fixed max number of regions, but would improve readability. Rob > + /* get the next reserved block */ > + rstart =3D be32_to_cpu(*reserved_list++); > + rsize =3D be32_to_cpu(*reserved_list++); > + > + /* catch unsorted list entries */ > + if (rstart < cur_start) { > + dev_err(&pdev->dev, > + "unsorted reserved list (0x%x bef= ore current 0x%x)\n", > + rstart, cur_start); > + if (sram->clk) > + clk_disable_unprepare(sram->clk); > + return -EINVAL; > + } > + > + dev_dbg(&pdev->dev, > + "found reserved block 0x%x-0x%x\n", > + rstart, rstart + rsize); > + } else { > + /* the last chunk extends to the end of the regio= n */ > + rstart =3D size; > + rsize =3D 0; > + } > + > + /* current start is in a reserved block */ > + if (rstart <=3D cur_start) { > + cur_start =3D rstart + rsize; > + continue; > + } > + > + /* > + * allocate the space between the current starting > + * address and the following reserved block, or the > + * end of the region. > + */ > + cur_size =3D rstart - cur_start; > + > + dev_dbg(&pdev->dev, "adding chunk 0x%x-0x%x\n", > + cur_start, cur_start + cur_size); > + ret =3D gen_pool_add_virt(sram->pool, > + (unsigned long)virt_base + cur_start, > + res->start + cur_start, cur_size, -1); > + if (ret < 0) { > + if (sram->clk) > + clk_disable_unprepare(sram->clk); > + return ret; > + } > + > + /* next allocation after this reserved block */ > + cur_start =3D rstart + rsize; > } > > platform_set_drvdata(pdev, sram); > -- > 1.7.10.4 >