From: robherring2@gmail.com (Rob Herring)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH RFC] of: add a basic memory driver
Date: Wed, 11 Sep 2013 11:40:48 -0500 [thread overview]
Message-ID: <52309D10.9020708@gmail.com> (raw)
In-Reply-To: <1378863781-4235-1-git-send-email-emilio@elopez.com.ar>
On 09/10/2013 08:43 PM, Emilio L?pez wrote:
> This driver's only job is to claim and ensure that the necessary clock
> for memory operation on a DT-enabled machine remains enabled.
>
> Signed-off-by: Emilio L?pez <emilio@elopez.com.ar>
> ---
>
> Hi,
>
> I am currently facing an issue with the clock setup: a critical but
> unclaimed clock gets disabled as a side effect of disabling one of its
> children. The clock setup looks something like this:
>
> PLL
> |
> ------------
> | |
> DDR Others
> |
> periph
This would be more accurate:
PLL
|
----------------
| |
DDR Ctrlr Others
|
DDR
So having a DDR controller node with a clock is the right way to do
this. There are other possible needs for describing the DDR controller
such as low power modes and ECC.
Rob
>
> The PLL clock is marked with the CLK_IGNORE_UNUSED flag, so on a normal
> boot it remains on, even after the unused clocks cleanup code runs. The
> problem occurs when someone enables "periph" and then, later on, disables
> it: the framework starts disabling clocks upwards on the tree,
> eventually switching the PLL off (and that kills the machine, as the memory
> clock is shut down).
>
> There's two possible solutions I can think of:
> 1) add some extra checks on the framework to not turn off clocks marked
> with such a flag on the non-explicit case (ie, when I'm disabling
> some other clock)
> 2) create an actual user of the DDR clock, that way it won't get
> disabled simply because it's being used.
>
> I considered 1) and implemented it, but the result was not pretty. This
> patch is my take on 2). Please let me know what you think; all feedback
> is welcome :)
>
> Cheers,
>
> Emilio
>
> drivers/of/Kconfig | 6 ++++++
> drivers/of/Makefile | 1 +
> drivers/of/of_memory.c | 30 ++++++++++++++++++++++++++++++
> 3 files changed, 37 insertions(+)
> create mode 100644 drivers/of/of_memory.c
>
> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
> index 9d2009a..f6c5e20 100644
> --- a/drivers/of/Kconfig
> +++ b/drivers/of/Kconfig
> @@ -80,4 +80,10 @@ config OF_RESERVED_MEM
> help
> Initialization code for DMA reserved memory
>
> +config OF_MEMORY
> + depends on COMMON_CLK
> + def_bool y
> + help
> + Simple memory initialization
> +
> endmenu # OF
> diff --git a/drivers/of/Makefile b/drivers/of/Makefile
> index ed9660a..15f0167 100644
> --- a/drivers/of/Makefile
> +++ b/drivers/of/Makefile
> @@ -10,3 +10,4 @@ obj-$(CONFIG_OF_PCI) += of_pci.o
> obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o
> obj-$(CONFIG_OF_MTD) += of_mtd.o
> obj-$(CONFIG_OF_RESERVED_MEM) += of_reserved_mem.o
> +obj-$(CONFIG_OF_MEMORY) += of_memory.o
> diff --git a/drivers/of/of_memory.c b/drivers/of/of_memory.c
> new file mode 100644
> index 0000000..a833f7a
> --- /dev/null
> +++ b/drivers/of/of_memory.c
> @@ -0,0 +1,30 @@
> +/*
> + * Simple memory driver
> + */
> +
> +#include <linux/of.h>
> +#include <linux/clk.h>
> +
> +static int __init of_memory_enable(void)
> +{
> + struct device_node *np;
> + struct clk *clk;
> +
> + np = of_find_node_by_path("/memory");
> + if (!np) {
> + pr_err("no /memory on DT!\n");
> + return 0;
> + }
> +
> + clk = of_clk_get(np, 0);
> + if (!IS_ERR(clk)) {
> + clk_prepare_enable(clk);
> + clk_put(clk);
> + }
> +
> + of_node_put(np);
> +
> + return 0;
> +}
> +
> +device_initcall(of_memory_enable);
>
WARNING: multiple messages have this Message-ID (diff)
From: Rob Herring <robherring2@gmail.com>
To: "Emilio López" <emilio@elopez.com.ar>
Cc: devicetree@vger.kernel.org,
Mike Turquette <mturquette@linaro.org>,
rob.herring@calxeda.com, david.lanzendoerfer@o2s.ch,
grant.likely@linaro.org,
Maxime Ripard <maxime.ripard@free-electrons.com>,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH RFC] of: add a basic memory driver
Date: Wed, 11 Sep 2013 11:40:48 -0500 [thread overview]
Message-ID: <52309D10.9020708@gmail.com> (raw)
In-Reply-To: <1378863781-4235-1-git-send-email-emilio@elopez.com.ar>
On 09/10/2013 08:43 PM, Emilio López wrote:
> This driver's only job is to claim and ensure that the necessary clock
> for memory operation on a DT-enabled machine remains enabled.
>
> Signed-off-by: Emilio López <emilio@elopez.com.ar>
> ---
>
> Hi,
>
> I am currently facing an issue with the clock setup: a critical but
> unclaimed clock gets disabled as a side effect of disabling one of its
> children. The clock setup looks something like this:
>
> PLL
> |
> ------------
> | |
> DDR Others
> |
> periph
This would be more accurate:
PLL
|
----------------
| |
DDR Ctrlr Others
|
DDR
So having a DDR controller node with a clock is the right way to do
this. There are other possible needs for describing the DDR controller
such as low power modes and ECC.
Rob
>
> The PLL clock is marked with the CLK_IGNORE_UNUSED flag, so on a normal
> boot it remains on, even after the unused clocks cleanup code runs. The
> problem occurs when someone enables "periph" and then, later on, disables
> it: the framework starts disabling clocks upwards on the tree,
> eventually switching the PLL off (and that kills the machine, as the memory
> clock is shut down).
>
> There's two possible solutions I can think of:
> 1) add some extra checks on the framework to not turn off clocks marked
> with such a flag on the non-explicit case (ie, when I'm disabling
> some other clock)
> 2) create an actual user of the DDR clock, that way it won't get
> disabled simply because it's being used.
>
> I considered 1) and implemented it, but the result was not pretty. This
> patch is my take on 2). Please let me know what you think; all feedback
> is welcome :)
>
> Cheers,
>
> Emilio
>
> drivers/of/Kconfig | 6 ++++++
> drivers/of/Makefile | 1 +
> drivers/of/of_memory.c | 30 ++++++++++++++++++++++++++++++
> 3 files changed, 37 insertions(+)
> create mode 100644 drivers/of/of_memory.c
>
> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
> index 9d2009a..f6c5e20 100644
> --- a/drivers/of/Kconfig
> +++ b/drivers/of/Kconfig
> @@ -80,4 +80,10 @@ config OF_RESERVED_MEM
> help
> Initialization code for DMA reserved memory
>
> +config OF_MEMORY
> + depends on COMMON_CLK
> + def_bool y
> + help
> + Simple memory initialization
> +
> endmenu # OF
> diff --git a/drivers/of/Makefile b/drivers/of/Makefile
> index ed9660a..15f0167 100644
> --- a/drivers/of/Makefile
> +++ b/drivers/of/Makefile
> @@ -10,3 +10,4 @@ obj-$(CONFIG_OF_PCI) += of_pci.o
> obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o
> obj-$(CONFIG_OF_MTD) += of_mtd.o
> obj-$(CONFIG_OF_RESERVED_MEM) += of_reserved_mem.o
> +obj-$(CONFIG_OF_MEMORY) += of_memory.o
> diff --git a/drivers/of/of_memory.c b/drivers/of/of_memory.c
> new file mode 100644
> index 0000000..a833f7a
> --- /dev/null
> +++ b/drivers/of/of_memory.c
> @@ -0,0 +1,30 @@
> +/*
> + * Simple memory driver
> + */
> +
> +#include <linux/of.h>
> +#include <linux/clk.h>
> +
> +static int __init of_memory_enable(void)
> +{
> + struct device_node *np;
> + struct clk *clk;
> +
> + np = of_find_node_by_path("/memory");
> + if (!np) {
> + pr_err("no /memory on DT!\n");
> + return 0;
> + }
> +
> + clk = of_clk_get(np, 0);
> + if (!IS_ERR(clk)) {
> + clk_prepare_enable(clk);
> + clk_put(clk);
> + }
> +
> + of_node_put(np);
> +
> + return 0;
> +}
> +
> +device_initcall(of_memory_enable);
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2013-09-11 16:40 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-09-11 1:43 [PATCH RFC] of: add a basic memory driver Emilio López
2013-09-11 1:43 ` Emilio López
2013-09-11 7:54 ` Maxime Ripard
2013-09-11 7:54 ` Maxime Ripard
2013-09-11 9:34 ` Emilio López
2013-09-11 9:34 ` Emilio López
2013-09-12 0:21 ` Mike Turquette
2013-09-12 0:21 ` Mike Turquette
2013-09-11 16:40 ` Rob Herring [this message]
2013-09-11 16:40 ` Rob Herring
2013-09-13 0:30 ` [PATCH] memory: add a basic OF-based " Emilio López
2013-09-13 0:30 ` Emilio López
2013-09-13 0:30 ` Emilio López
2013-09-13 0:57 ` Olof Johansson
2013-09-13 0:57 ` Olof Johansson
2013-09-13 0:57 ` Olof Johansson
2013-09-13 1:31 ` Emilio López
2013-09-13 1:31 ` Emilio López
2013-09-13 1:31 ` Emilio López
2013-09-13 14:00 ` Rob Herring
2013-09-13 14:00 ` Rob Herring
2013-09-13 15:49 ` Olof Johansson
2013-09-13 15:49 ` Olof Johansson
2013-09-13 15:49 ` Olof Johansson
2013-09-15 12:43 ` Grant Likely
2013-09-15 12:43 ` Grant Likely
2013-09-16 12:55 ` Rob Herring
2013-09-16 12:55 ` Rob Herring
2013-09-16 12:55 ` Rob Herring
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=52309D10.9020708@gmail.com \
--to=robherring2@gmail.com \
--cc=linux-arm-kernel@lists.infradead.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.