[PATCH 0/4] add minimal bcm2835-sdram driver

[PATCH 0/4] add minimal bcm2835-sdram driver

[kernel]

From: Martin Sperl <kernel@martin.sperl.org>

As the sdram clock is a critical clock to the system
the minimal bcm2835-sdram driver claims (and enables)
this clock and also exposes the corresponding sdram
registers via debugfs.

Martin Sperl (4):
  dt: bindings: add bcm2835-memory-controller documentation
  memory: bcm2835: add bcm2835-memory controller
  ARM: dts: bcm2835: add the bcm2835-sdram-controller to the dt
  ARM: bcm2835_defconfig: add bcm2835-sdram controller

 .../memory-controllers/brcm,bcm2835-sdram.txt      |  16 +++
 arch/arm/boot/dts/bcm283x.dtsi                     |   6 +
 arch/arm/configs/bcm2835_defconfig                 |   1 +
 drivers/memory/Kconfig                             |   7 +
 drivers/memory/Makefile                            |   1 +
 drivers/memory/bcm2835-sdram.c                     | 152 +++++++++++++++++++++
 6 files changed, 183 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/memory-controllers/brcm,bcm2835-sdram.txt
 create mode 100644 drivers/memory/bcm2835-sdram.c

[PATCH 1/4] dt: bindings: add bcm2835-memory-controller documentation

[kernel]

From: Martin Sperl <kernel@martin.sperl.org>

Add memory controller dt-binding documentation for the
bcm2835 soc.

Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
---
 .../bindings/memory-controllers/brcm,bcm2835-sdram.txt   | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/memory-controllers/brcm,bcm2835-sdram.txt

diff --git a/Documentation/devicetree/bindings/memory-controllers/brcm,bcm2835-sdram.txt b/Documentation/devicetree/bindings/memory-controllers/brcm,bcm2835-sdram.txt
new file mode 100644
index 0000000..d1cb151
--- /dev/null
+++ b/Documentation/devicetree/bindings/memory-controllers/brcm,bcm2835-sdram.txt
@@ -0,0 +1,16 @@
+Binding for SDRAM driver for BCM2835 SoCs.
+
+Required parameters:
+-------------------
+
+compatible : 	should be: "brcm,bcm2835-sdram"
+reg :		Address ranges of the ic1/ic2 registers.
+clocks : 	Phandle of the clock used by the sdram.
+
+Example:
+
+memory-conroller@7e002000 {
+	compatible = "brcm,bcm2835-sdram";
+	reg = <0x7e002000 0x58>, <0x7e002800 0x58>;
+	clocks = <&clocks BCM2835_CLOCK_SDRAM>;
+};
-- 
2.1.4

Re: [PATCH 1/4] dt: bindings: add bcm2835-memory-controller documentation

[Rob Herring]

On Thu, May 12, 2016 at 12:38:49PM +0000, kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org wrote:
> From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
> 
> Add memory controller dt-binding documentation for the
> bcm2835 soc.
> 
> Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
> ---
>  .../bindings/memory-controllers/brcm,bcm2835-sdram.txt   | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/memory-controllers/brcm,bcm2835-sdram.txt

I'd suspect a DDR controller has more than one clock, but otherwise 
looks fine. And this binding is pretty much independent of the 
discussion for this series.

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/4] memory: bcm2835: add bcm2835-memory controller

[kernel]

From: Martin Sperl <kernel@martin.sperl.org>

Add a memory-controller driver for the bcm2835 SOC.

This is mostly needed to claim the SDRAM clock
so that this (and the corresponding parent pll)
never gets disabled.

It also exposes the sdram registers via debugfs.

Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
---
 drivers/memory/Kconfig         |   7 ++
 drivers/memory/Makefile        |   1 +
 drivers/memory/bcm2835-sdram.c | 152 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 160 insertions(+)
 create mode 100644 drivers/memory/bcm2835-sdram.c

diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig
index 51d5cd2..a55cad3 100644
--- a/drivers/memory/Kconfig
+++ b/drivers/memory/Kconfig
@@ -25,6 +25,13 @@ config ATMEL_SDRAMC
 	  Starting with the at91sam9g45, this controller supports SDR, DDR and
 	  LP-DDR memories.
 
+config BCM2835_SDRAM
+	bool "Broadcom BCM2835 SDRAM Controller"
+	default y
+	depends on ARCH_BCM2835 || COMPILE_TEST
+	help
+	  This driver is for Broadcom BCM2835 SDRAM Controller.
+
 config TI_AEMIF
 	tristate "Texas Instruments AEMIF driver"
 	depends on (ARCH_DAVINCI || ARCH_KEYSTONE) && OF
diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile
index 890bdf4..1287b90 100644
--- a/drivers/memory/Makefile
+++ b/drivers/memory/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_OF)		+= of_memory.o
 endif
 obj-$(CONFIG_ARM_PL172_MPMC)	+= pl172.o
 obj-$(CONFIG_ATMEL_SDRAMC)	+= atmel-sdramc.o
+obj-$(CONFIG_BCM2835_SDRAM)	+= bcm2835-sdram.o
 obj-$(CONFIG_TI_AEMIF)		+= ti-aemif.o
 obj-$(CONFIG_TI_EMIF)		+= emif.o
 obj-$(CONFIG_OMAP_GPMC)		+= omap-gpmc.o
diff --git a/drivers/memory/bcm2835-sdram.c b/drivers/memory/bcm2835-sdram.c
new file mode 100644
index 0000000..7c9c0ca
--- /dev/null
+++ b/drivers/memory/bcm2835-sdram.c
@@ -0,0 +1,152 @@
+/*
+ * Driver for Broadcom BCM2835 soc sdram controller
+ *
+ * Copyright (C) 2016 Martin Sperl
+ *
+ * inspired by: atmel-sdramc
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk.h>
+#include <linux/debugfs.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+
+struct bcm2835_sdram_data {
+	void __iomem *regs[2];
+	struct dentry *debugfsdir;
+	struct clk *clk;
+};
+
+#define R(n, o) { .name = n, .offset = o }
+static const struct debugfs_reg32 bcm2835_sdram_regs[] = {
+	R("c",			0x00),
+	R("s",			0x04),
+	R("src0",		0x08),
+	R("src1",		0x0c),
+	R("mask0",		0x10),
+	R("mask1",		0x14),
+	R("mask2",		0x18),
+	R("mask3",		0x1c),
+	R("mask4",		0x20),
+	R("mask5",		0x24),
+	R("mask6",		0x28),
+	R("mask7",		0x2c),
+	R("vaddr",		0x30),
+	R("wakeup",		0x34),
+	R("profile",		0x38),
+	/* 0x3c is not defined */
+	R("force0",		0x40),
+	R("force1",		0x44),
+	/* 0x48 to 0x54 are write only */
+};
+
+static void bcm2835_sdram_debugfs(struct platform_device *pdev)
+{
+	struct bcm2835_sdram_data *data = platform_get_drvdata(pdev);
+	struct debugfs_regset32 *regset;
+	char *name;
+	int i;
+
+	data->debugfsdir = debugfs_create_dir("bcm2835_sdram", NULL);
+	if (!data->debugfsdir)
+		return;
+
+	/* create the regsets */
+	for (i = 0; i < 2; i++) {
+		name = devm_kasprintf(&pdev->dev, GFP_KERNEL,
+				      "regset%d", i);
+		if (!name)
+			return;
+
+		regset = devm_kzalloc(&pdev->dev, sizeof(*regset),
+				      GFP_KERNEL);
+		if (!regset)
+			return;
+
+		regset->regs = bcm2835_sdram_regs;
+		regset->nregs = ARRAY_SIZE(bcm2835_sdram_regs);
+		regset->base = data->regs[i];
+
+		debugfs_create_regset32(name, S_IRUGO,
+					data->debugfsdir, regset);
+	}
+}
+
+static int bcm2835_sdram_probe(struct platform_device *pdev)
+{
+	struct bcm2835_sdram_data *data;
+	struct resource *res;
+	int err, i;
+
+	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+	platform_set_drvdata(pdev, data);
+
+	/* get registers */
+	for (i = 0; i < 2; i++) {
+		res = platform_get_resource(pdev, IORESOURCE_MEM, i);
+		data->regs[i] = devm_ioremap_resource(&pdev->dev, res);
+		if (IS_ERR(data->regs[i])) {
+			err = PTR_ERR(data->regs[i]);
+			dev_err(&pdev->dev,
+				"Could not get register set %d: %d\n",
+				i, err);
+			return err;
+		}
+	}
+	/* get clock */
+	data->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(data->clk))
+		return PTR_ERR(data->clk);
+	clk_prepare_enable(data->clk);
+
+	bcm2835_sdram_debugfs(pdev);
+
+	return 0;
+}
+
+static int bcm2835_sdram_remove(struct platform_device *pdev)
+{
+	struct bcm2835_sdram_data *data = platform_get_drvdata(pdev);
+
+	debugfs_remove_recursive(data->debugfsdir);
+
+	return 0;
+}
+
+static const struct of_device_id bcm2835_sdram_of_match_table[] = {
+	{ .compatible = "brcm,bcm2835-sdram", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, bcm2835_sdram_of_match_table);
+
+static struct platform_driver bcm2835_sdram_driver = {
+	.probe = bcm2835_sdram_probe,
+	.remove = bcm2835_sdram_remove,
+	.driver = {
+		.name = "bcm2835_sdram",
+		.of_match_table = bcm2835_sdram_of_match_table,
+	},
+};
+module_platform_driver(bcm2835_sdram_driver);
+
+MODULE_AUTHOR("Martin Sperl");
+MODULE_DESCRIPTION("sdram driver for bcm2835 chip");
+MODULE_LICENSE("GPL");
-- 
2.1.4

[PATCH 3/4] ARM: dts: bcm2835: add the bcm2835-sdram-controller to the dt

[kernel]

From: Martin Sperl <kernel@martin.sperl.org>

Add the bcm2835 sdram controller to the device tree.

Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
---
 arch/arm/boot/dts/bcm283x.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/bcm283x.dtsi b/arch/arm/boot/dts/bcm283x.dtsi
index 8aaf193..9db9d97 100644
--- a/arch/arm/boot/dts/bcm283x.dtsi
+++ b/arch/arm/boot/dts/bcm283x.dtsi
@@ -22,6 +22,12 @@
 		#address-cells = <1>;
 		#size-cells = <1>;
 
+		memory-controller@7e002000 {
+			compatible = "brcm,bcm2835-sdram";
+			reg = <0x7e002000 0x58>, <0x7e002800 0x58>;
+			clocks = <&clocks BCM2835_CLOCK_SDRAM>;
+		};
+
 		timer@7e003000 {
 			compatible = "brcm,bcm2835-system-timer";
 			reg = <0x7e003000 0x1000>;
-- 
2.1.4

Re: [PATCH 3/4] ARM: dts: bcm2835: add the bcm2835-sdram-controller to the dt

[Stefan Wahren]

Hi,

> kernel@martin.sperl.org hat am 12. Mai 2016 um 14:38 geschrieben:
> 
> 
> From: Martin Sperl <kernel@martin.sperl.org>
> 
> Add the bcm2835 sdram controller to the device tree.
> 
> Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
> ---
>  arch/arm/boot/dts/bcm283x.dtsi | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/bcm283x.dtsi b/arch/arm/boot/dts/bcm283x.dtsi
> index 8aaf193..9db9d97 100644
> --- a/arch/arm/boot/dts/bcm283x.dtsi
> +++ b/arch/arm/boot/dts/bcm283x.dtsi
> @@ -22,6 +22,12 @@
>  		#address-cells = <1>;
>  		#size-cells = <1>;
>  
> +		memory-controller@7e002000 {
> +			compatible = "brcm,bcm2835-sdram";
> +			reg = <0x7e002000 0x58>, <0x7e002800 0x58>;

where do you get these addresses?

According to register documentation [1] the adresses belong to the interrupt
controller.

Maybe 0x7ee00000 is the right address.

[1] - https://github.com/hermanhermitage/videocoreiv/wiki/Register-Documentation

> +			clocks = <&clocks BCM2835_CLOCK_SDRAM>;
> +		};
> +
>  		timer@7e003000 {
>  			compatible = "brcm,bcm2835-system-timer";
>  			reg = <0x7e003000 0x1000>;
> -- 
> 2.1.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/4] ARM: dts: bcm2835: add the bcm2835-sdram-controller to the dt

[Martin Sperl]

> On 12.05.2016, at 16:56, Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> wrote:
> 
> Hi,
> 
>> kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org hat am 12. Mai 2016 um 14:38 geschrieben:
>> 
>> 
>> From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
>> 
>> Add the bcm2835 sdram controller to the device tree.
>> 
>> Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
>> ---
>> arch/arm/boot/dts/bcm283x.dtsi | 6 ++++++
>> 1 file changed, 6 insertions(+)
>> 
>> diff --git a/arch/arm/boot/dts/bcm283x.dtsi b/arch/arm/boot/dts/bcm283x.dtsi
>> index 8aaf193..9db9d97 100644
>> --- a/arch/arm/boot/dts/bcm283x.dtsi
>> +++ b/arch/arm/boot/dts/bcm283x.dtsi
>> @@ -22,6 +22,12 @@
>> 		#address-cells = <1>;
>> 		#size-cells = <1>;
>> 
>> +		memory-controller@7e002000 {
>> +			compatible = "brcm,bcm2835-sdram";
>> +			reg = <0x7e002000 0x58>, <0x7e002800 0x58>;
> 
> where do you get these addresses?
I took this info based on the shared headers by broadcom:
https://github.com/msperl/rpi-registers/blob/master/md/README.md

And the bootrom code showed something similar that would point to
these registers.

Ic0 and ic1 register values also show the valid memory regions:
Especially IC[01]_SRC0 contains reference to the SRAM
size on the CM - 0x20000000
here the fump of devmem2: 
Value at address 0x3F00200C (0x76f3300c): 0x20000000.

But as you point out that may also be the interrupt controller for the VC4
cores…

It is probably the location where the VC4 interrupt table is situated.


> According to register documentation [1] the adresses belong to the interrupt
> controller.
> 
> Maybe 0x7ee00000 is the right address.
This would also fall in range with the cache register ranges:
0x7ee00000 SD
0x7ee01000 L2
0x7ee02000 L1 

L1 contains also IC0 and IC1 register names (L1_IC0_CONTROL et.al.)
so I guess this is what has set me in the wrong direction...

Thanks for pointing it out...

I guess I will need to amend the patchset for that…

Martin


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/4] ARM: bcm2835_defconfig: add bcm2835-sdram controller

[kernel]

From: Martin Sperl <kernel@martin.sperl.org>

Add the bcm2835-sdram controller driver to the default config.
(as the driver is enabled by default vi KConfig, we only need
to enable CONFIG_MEMORY)

Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
---
 arch/arm/configs/bcm2835_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/bcm2835_defconfig b/arch/arm/configs/bcm2835_defconfig
index 1ef69fc..29a44bd 100644
--- a/arch/arm/configs/bcm2835_defconfig
+++ b/arch/arm/configs/bcm2835_defconfig
@@ -107,6 +107,7 @@ CONFIG_MAILBOX=y
 CONFIG_BCM2835_MBOX=y
 # CONFIG_IOMMU_SUPPORT is not set
 CONFIG_RASPBERRYPI_POWER=y
+CONFIG_MEMORY=y
 CONFIG_PWM=y
 CONFIG_PWM_BCM2835=y
 CONFIG_RASPBERRYPI_FIRMWARE=y

Re: [PATCH 0/4] add minimal bcm2835-sdram driver

[Stefan Wahren]

Hi Martin,

> kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org hat am 12. Mai 2016 um 14:38 geschrieben:
> 
> 
> From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
> 
> As the sdram clock is a critical clock to the system
> the minimal bcm2835-sdram driver claims (and enables)
> this clock and also exposes the corresponding sdram
> registers via debugfs.

sounds like this driver should fix an clock handling issue. Unfortunately this
isn't a solution in case the driver is disabled.

Does the GPU firmware handle the SDRAM controller or is it initialized by
bootcode?

Stefan
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/4] add minimal bcm2835-sdram driver

[Martin Sperl]

> On 12.05.2016, at 16:50, Stefan Wahren <stefan.wahren@i2se.com> wrote:
> 
> Hi Martin,
> 
>> kernel@martin.sperl.org hat am 12. Mai 2016 um 14:38 geschrieben:
>> 
>> 
>> From: Martin Sperl <kernel@martin.sperl.org>
>> 
>> As the sdram clock is a critical clock to the system
>> the minimal bcm2835-sdram driver claims (and enables)
>> this clock and also exposes the corresponding sdram
>> registers via debugfs.
> 
> sounds like this driver should fix an clock handling issue. Unfortunately this
> isn't a solution in case the driver is disabled.
Unfortunately there is no way around this - the driver has
to be enabled so that the sdram clock or the parent pll,
which typically is plld_core, never gets disabled.

The only other option would be marking the clock as critical
for those legacy drivers.

See also the discussions around the clock register for the
sdram clock, where we have the “normal” clock and some
pll as well - even if the “normal” clock is disabled, 
then clearing the sdram register (or the parent) freezes
the system.

> 
> Does the GPU firmware handle the SDRAM controller or is it initialized by
> bootcode?
> 

AFAIK it is enabled in bootcode.bin and - as of now - the firmware
updates refresh cycles when SOC temperatures change.
FW checks every 30 seconds unless there is a key set in config.txt,
which - supposedly - produces a slight impact every 30 seconds to
the system.

Martin

Re: [PATCH 0/4] add minimal bcm2835-sdram driver

[Stefan Wahren]

Hi,

> Martin Sperl <kernel@martin.sperl.org> hat am 12. Mai 2016 um 17:28
> geschrieben:
> 
> 
> 
> > On 12.05.2016, at 16:50, Stefan Wahren <stefan.wahren@i2se.com> wrote:
> > 
> > Hi Martin,
> > 
> >> kernel@martin.sperl.org hat am 12. Mai 2016 um 14:38 geschrieben:
> >> 
> >> 
> >> From: Martin Sperl <kernel@martin.sperl.org>
> >> 
> >> As the sdram clock is a critical clock to the system
> >> the minimal bcm2835-sdram driver claims (and enables)
> >> this clock and also exposes the corresponding sdram
> >> registers via debugfs.
> > 
> > sounds like this driver should fix an clock handling issue. Unfortunately
> > this
> > isn't a solution in case the driver is disabled.
> Unfortunately there is no way around this - the driver has
> to be enabled so that the sdram clock or the parent pll,
> which typically is plld_core, never gets disabled.
> 
> The only other option would be marking the clock as critical
> for those legacy drivers.

i would prefer this option. Since this would be more a fix we could get this
faster in, it's more clear why we are doing that and less to review.

Did i miss a drawback?

Stefan

> 
> See also the discussions around the clock register for the
> sdram clock, where we have the “normal” clock and some
> pll as well - even if the “normal” clock is disabled, 
> then clearing the sdram register (or the parent) freezes
> the system.
> 
> > 
> > Does the GPU firmware handle the SDRAM controller or is it initialized by
> > bootcode?
> > 
> 
> AFAIK it is enabled in bootcode.bin and - as of now - the firmware
> updates refresh cycles when SOC temperatures change.
> FW checks every 30 seconds unless there is a key set in config.txt,
> which - supposedly - produces a slight impact every 30 seconds to
> the system.
> 
> Martin

Re: [PATCH 0/4] add minimal bcm2835-sdram driver

[Martin Sperl]

> On 12.05.2016, at 17:55, Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> wrote:
> 
> Hi,
> 
>> Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org> hat am 12. Mai 2016 um 17:28
>> geschrieben:
>> 
>> 
>> 
>>> On 12.05.2016, at 16:50, Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> wrote:
>>> 
>>> Hi Martin,
>>> 
>>>> kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org hat am 12. Mai 2016 um 14:38 geschrieben:
>>>> 
>>>> 
>>>> From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
>>>> 
>>>> As the sdram clock is a critical clock to the system
>>>> the minimal bcm2835-sdram driver claims (and enables)
>>>> this clock and also exposes the corresponding sdram
>>>> registers via debugfs.
>>> 
>>> sounds like this driver should fix an clock handling issue. Unfortunately
>>> this
>>> isn't a solution in case the driver is disabled.
>> Unfortunately there is no way around this - the driver has
>> to be enabled so that the sdram clock or the parent pll,
>> which typically is plld_core, never gets disabled.
>> 
>> The only other option would be marking the clock as critical
>> for those legacy drivers.
> 
> i would prefer this option. Since this would be more a fix we could get this
> faster in, it's more clear why we are doing that and less to review.
> 
> Did i miss a drawback?

We had long discussions already on that - and HAND_OFF did not make
it into the kernel either.

I had hoped that this was simple enough.

Martin



--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/4] add minimal bcm2835-sdram driver

[Eric Anholt]

[-- Attachment #1: Type: text/plain, Size: 631 bytes --]

kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org writes:

> From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
>
> As the sdram clock is a critical clock to the system
> the minimal bcm2835-sdram driver claims (and enables)
> this clock and also exposes the corresponding sdram
> registers via debugfs.

I don't think this is a good solution to the problem you are trying to
work around.  You're relying on the fact that this driver gets
successfully probed before a driver that would -EPROBE_DEFER on a
sibling clock, which is not a guarantee.

Let's please continue the debugging of clock management on linux-clk.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: [PATCH 0/4] add minimal bcm2835-sdram driver

[Martin Sperl]

> On 12.05.2016, at 20:15, Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org> wrote:
> 
> kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org writes:
> 
>> From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
>> 
>> As the sdram clock is a critical clock to the system
>> the minimal bcm2835-sdram driver claims (and enables)
>> this clock and also exposes the corresponding sdram
>> registers via debugfs.
> 
> I don't think this is a good solution to the problem you are trying to
> work around.  You're relying on the fact that this driver gets
> successfully probed before a driver that would -EPROBE_DEFER on a
> sibling clock, which is not a guarantee.
Ah - that is probably the reason why hand_off has not gone in...

> 
> Let's please continue the debugging of clock management on linux-clk.

I am just trying to find a solution that works...

Still I think that having a driver that claims the clock and register range
is good to have - at least this way we can have a peek at what
the firmware does set up.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html