From: Grant Likely <grant.likely@secretlab.ca>
To: linux-kernel@vger.kernel.org, Arnd Bergmann <arnd@arndb.de>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Rob Herring <rob.herring@calxeda.com>,
Paul Gortmaker <paul.gortmaker@windriver.com>,
Shawn Guo <shawn.guo@linaro.org>,
Richard Zhao <richard.zhao@freescale.com>,
Huang Shijie <shijie8@gmail.com>,
Dong Aisheng <dong.aisheng@linaro.org>,
Matt Porter <mporter@ti.com>,
kernel@pengutronix.de, devicetree-discuss@lists.ozlabs.org
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Subject: Re: [PATCH v5 4/4] misc: sram: add support for configurable allocation order
Date: Wed, 14 Nov 2012 19:15:51 +0000 [thread overview]
Message-ID: <20121114191551.5E8673E0B7C@localhost> (raw)
In-Reply-To: <1350570453-24546-5-git-send-email-p.zabel@pengutronix.de>
On Thu, 18 Oct 2012 16:27:33 +0200, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> From: Matt Porter <mporter@ti.com>
>
> Adds support for setting the genalloc pool's minimum allocation
> order via DT or platform data. The allocation order is optional
> for both the DT property and platform data case. If it is not
> present then the order defaults to PAGE_SHIFT to preserve the
> current behavior.
>
> Signed-off-by: Matt Porter <mporter@ti.com>
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
> Documentation/devicetree/bindings/misc/sram.txt | 12 ++++++++++-
> drivers/misc/sram.c | 14 ++++++++++++-
> include/linux/platform_data/sram.h | 25 +++++++++++++++++++++++
> 3 files changed, 49 insertions(+), 2 deletions(-)
> create mode 100644 include/linux/platform_data/sram.h
>
> diff --git a/Documentation/devicetree/bindings/misc/sram.txt b/Documentation/devicetree/bindings/misc/sram.txt
> index b64136c..b1705ec 100644
> --- a/Documentation/devicetree/bindings/misc/sram.txt
> +++ b/Documentation/devicetree/bindings/misc/sram.txt
> @@ -8,10 +8,20 @@ Required properties:
>
> - reg : SRAM iomem address range
>
> -Example:
> +Optional properties:
> +
> +- alloc-order : Minimum allocation order for the SRAM pool
Looks okay, but I think the property name is confusing. I for one had
no idea what 'order' would be and why it was important. I had to read
the code to figure it out.
It does raise the question though of what is this binding actually
for? Does it reflect a limitation of the SRAM? or of the hardware using
the SRAM? Or is it an optimization? How do you expect to use it?
Assuming it is appropriate to put into the device tree, I'd suggest a
different name. Instead of 'order', how about 'sram-alloc-align' (in
address bits) or 'sram-alloc-min-size' (in bytes).
> @@ -60,7 +62,17 @@ static int __devinit sram_probe(struct platform_device *pdev)
> if (!IS_ERR(sram->clk))
> clk_prepare_enable(sram->clk);
>
> - sram->pool = gen_pool_create(PAGE_SHIFT, -1);
> + if (pdev->dev.of_node)
> + of_property_read_u32(pdev->dev.of_node,
> + "alloc-order", &alloc_order);
> + else
> + if (pdev->dev.platform_data) {
> + struct sram_pdata *pdata = pdev->dev.platform_data;
> + if (pdata->alloc_order)
> + alloc_order = pdata->alloc_order;
> + }
> +
> + sram->pool = gen_pool_create(alloc_order, -1);
Do you already have a user for the platform_data use case? If not then
I'd drop it entirely and skip adding the new header file.
g.
next prev parent reply other threads:[~2012-11-14 19:15 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-10-18 14:27 [PATCH v5 0/4] Add generic driver for on-chip SRAM Philipp Zabel
2012-10-18 14:27 ` [PATCH v5 1/4] genalloc: add a global pool list, allow to find pools by phys address Philipp Zabel
2012-10-25 18:56 ` Greg Kroah-Hartman
[not found] ` <1350570453-24546-2-git-send-email-p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2012-10-26 19:46 ` Paul Gortmaker
2012-11-15 13:25 ` Philipp Zabel
[not found] ` <1352985943.2399.198.camel-/rZezPiN1rtR6QfukMTsflXZhhPuCNm+@public.gmane.org>
2012-11-16 1:50 ` Paul Gortmaker
2012-10-18 14:27 ` [PATCH v5 2/4] misc: Generic on-chip SRAM allocation driver Philipp Zabel
2012-10-26 16:07 ` Paul Gortmaker
2012-10-29 12:20 ` Philipp Zabel
[not found] ` <1351513256.5872.103.camel-/rZezPiN1rtR6QfukMTsflXZhhPuCNm+@public.gmane.org>
2012-11-06 18:43 ` Paul Gortmaker
2012-10-18 14:27 ` [PATCH v5 3/4] misc: sram: Add optional clock Philipp Zabel
2012-10-26 15:18 ` Paul Gortmaker
2012-10-29 12:20 ` Philipp Zabel
2012-10-26 16:17 ` Paul Gortmaker
2012-10-29 12:20 ` Philipp Zabel
[not found] ` <1351513257.5872.104.camel-/rZezPiN1rtR6QfukMTsflXZhhPuCNm+@public.gmane.org>
2012-11-06 18:28 ` Paul Gortmaker
2012-10-18 14:27 ` [PATCH v5 4/4] misc: sram: add support for configurable allocation order Philipp Zabel
2012-11-14 19:15 ` Grant Likely [this message]
2012-11-15 13:11 ` Philipp Zabel
2012-11-15 16:52 ` Grant Likely
[not found] ` <1352985095.2399.184.camel-/rZezPiN1rtR6QfukMTsflXZhhPuCNm+@public.gmane.org>
2012-11-16 14:09 ` Matt Porter
2012-11-16 14:11 ` Grant Likely
2012-11-16 15:58 ` Philipp Zabel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20121114191551.5E8673E0B7C@localhost \
--to=grant.likely@secretlab.ca \
--cc=arnd@arndb.de \
--cc=devicetree-discuss@lists.ozlabs.org \
--cc=dong.aisheng@linaro.org \
--cc=gregkh@linuxfoundation.org \
--cc=kernel@pengutronix.de \
--cc=linux-kernel@vger.kernel.org \
--cc=mporter@ti.com \
--cc=p.zabel@pengutronix.de \
--cc=paul.gortmaker@windriver.com \
--cc=richard.zhao@freescale.com \
--cc=rob.herring@calxeda.com \
--cc=shawn.guo@linaro.org \
--cc=shijie8@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).