linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RESEND PATCH v2 0/6] Update Kona drivers to use clocks
@ 2013-10-16 21:47 Tim Kryger
  2013-10-16 21:47 ` [RESEND PATCH v2 1/6] ARM: dts: Declare clocks as fixed on bcm11351 Tim Kryger
                   ` (6 more replies)
  0 siblings, 7 replies; 22+ messages in thread
From: Tim Kryger @ 2013-10-16 21:47 UTC (permalink / raw)
  To: Christian Daudt, Rob Herring, Pawel Moll, Mark Rutland,
	Stephen Warren, Ian Campbell, Daniel Lezcano, Thomas Gleixner,
	Chris Ball
  Cc: Tim Kryger, devicetree, linux-arm-kernel, linux-kernel,
	linux-mmc, patches

This series declares the clocks present on bcm11351 (aka bcm281xx) SoCs
to be fixed at the rates configured by development bootloaders and then
updates the Kona drivers to make use of clock calls where appropriate.

This patch series depends on the following three commits being present:

https://lkml.org/lkml/2013/9/18/409
https://lkml.org/lkml/2013/9/20/305
http://www.spinics.net/lists/arm-kernel/msg274973.html

Changes in v2:
  - Renamed bsc4_clk to pmu_bsc_clk

Tim Kryger (6):
  ARM: dts: Declare clocks as fixed on bcm11351
  ARM: dts: Specify clocks for UARTs on bcm11351
  ARM: dts: Specify clocks for SDHCIs on bcm11351
  clocksource: kona: Add basic use of external clock
  ARM: dts: Specify clocks for timer on bcm11351
  mmc: sdhci-bcm-kona: Add basic use of clocks

 arch/arm/boot/dts/bcm11351.dtsi      | 111 +++++++++++++++++++++++++++++++++--
 drivers/clocksource/bcm_kona_timer.c |  14 ++++-
 drivers/mmc/host/sdhci-bcm-kona.c    |  30 +++++++++-
 3 files changed, 145 insertions(+), 10 deletions(-)

-- 
1.8.0.1



^ permalink raw reply	[flat|nested] 22+ messages in thread

* [RESEND PATCH v2 1/6] ARM: dts: Declare clocks as fixed on bcm11351
  2013-10-16 21:47 [RESEND PATCH v2 0/6] Update Kona drivers to use clocks Tim Kryger
@ 2013-10-16 21:47 ` Tim Kryger
  2013-11-08 10:53   ` Daniel Lezcano
  2013-10-16 21:47 ` [RESEND PATCH v2 2/6] ARM: dts: Specify clocks for UARTs " Tim Kryger
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 22+ messages in thread
From: Tim Kryger @ 2013-10-16 21:47 UTC (permalink / raw)
  To: Christian Daudt, Rob Herring, Pawel Moll, Mark Rutland,
	Stephen Warren, Ian Campbell, Daniel Lezcano, Thomas Gleixner,
	Chris Ball
  Cc: Tim Kryger, devicetree, linux-arm-kernel, linux-kernel,
	linux-mmc, patches

Declare clocks that are enabled and configured by bootloaders as fixed
rate clocks in the DTS such that device drivers may use standard clock
function calls.

Signed-off-by: Tim Kryger <tim.kryger@linaro.org>
Reviewed-by: Markus Mayer <markus.mayer@linaro.org>
Reviewed-by: Matt Porter <matt.porter@linaro.org>
---
 arch/arm/boot/dts/bcm11351.dtsi | 97 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 97 insertions(+)

diff --git a/arch/arm/boot/dts/bcm11351.dtsi b/arch/arm/boot/dts/bcm11351.dtsi
index 98e6bc0..c6464fb 100644
--- a/arch/arm/boot/dts/bcm11351.dtsi
+++ b/arch/arm/boot/dts/bcm11351.dtsi
@@ -126,4 +126,101 @@
 		status = "disabled";
 	};
 
+	clocks {
+		bsc1_clk: bsc1 {
+			compatible = "fixed-clock";
+			clock-frequency = <13000000>;
+			#clock-cells = <0>;
+		};
+
+		bsc2_clk: bsc2 {
+			compatible = "fixed-clock";
+			clock-frequency = <13000000>;
+			#clock-cells = <0>;
+		};
+
+		bsc3_clk: bsc3 {
+			compatible = "fixed-clock";
+			clock-frequency = <13000000>;
+			#clock-cells = <0>;
+		};
+
+		pmu_bsc_clk: pmu_bsc {
+			compatible = "fixed-clock";
+			clock-frequency = <13000000>;
+			#clock-cells = <0>;
+		};
+
+		hub_timer_clk: hub_timer {
+			compatible = "fixed-clock";
+			clock-frequency = <32768>;
+			#clock-cells = <0>;
+		};
+
+		pwm_clk: pwm {
+			compatible = "fixed-clock";
+			clock-frequency = <26000000>;
+			#clock-cells = <0>;
+		};
+
+		sdio1_clk: sdio1 {
+			compatible = "fixed-clock";
+			clock-frequency = <48000000>;
+			#clock-cells = <0>;
+		};
+
+		sdio2_clk: sdio2 {
+			compatible = "fixed-clock";
+			clock-frequency = <48000000>;
+			#clock-cells = <0>;
+		};
+
+		sdio3_clk: sdio3 {
+			compatible = "fixed-clock";
+			clock-frequency = <48000000>;
+			#clock-cells = <0>;
+		};
+
+		sdio4_clk: sdio4 {
+			compatible = "fixed-clock";
+			clock-frequency = <48000000>;
+			#clock-cells = <0>;
+		};
+
+		tmon_1m_clk: tmon_1m {
+			compatible = "fixed-clock";
+			clock-frequency = <1000000>;
+			#clock-cells = <0>;
+		};
+
+		uartb_clk: uartb {
+			compatible = "fixed-clock";
+			clock-frequency = <13000000>;
+			#clock-cells = <0>;
+		};
+
+		uartb2_clk: uartb2 {
+			compatible = "fixed-clock";
+			clock-frequency = <13000000>;
+			#clock-cells = <0>;
+		};
+
+		uartb3_clk: uartb3 {
+			compatible = "fixed-clock";
+			clock-frequency = <13000000>;
+			#clock-cells = <0>;
+		};
+
+		uartb4_clk: uartb4 {
+			compatible = "fixed-clock";
+			clock-frequency = <13000000>;
+			#clock-cells = <0>;
+		};
+
+		usb_otg_ahb_clk: usb_otg_ahb {
+			compatible = "fixed-clock";
+			clock-frequency = <52000000>;
+			#clock-cells = <0>;
+		};
+	};
 };
-- 
1.8.0.1



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [RESEND PATCH v2 2/6] ARM: dts: Specify clocks for UARTs on bcm11351
  2013-10-16 21:47 [RESEND PATCH v2 0/6] Update Kona drivers to use clocks Tim Kryger
  2013-10-16 21:47 ` [RESEND PATCH v2 1/6] ARM: dts: Declare clocks as fixed on bcm11351 Tim Kryger
@ 2013-10-16 21:47 ` Tim Kryger
  2013-10-17  5:41   ` Christian Daudt
  2013-10-17 13:56   ` Mark Rutland
  2013-10-16 21:47 ` [RESEND PATCH v2 3/6] ARM: dts: Specify clocks for SDHCIs " Tim Kryger
                   ` (4 subsequent siblings)
  6 siblings, 2 replies; 22+ messages in thread
From: Tim Kryger @ 2013-10-16 21:47 UTC (permalink / raw)
  To: Christian Daudt, Rob Herring, Pawel Moll, Mark Rutland,
	Stephen Warren, Ian Campbell, Daniel Lezcano, Thomas Gleixner,
	Chris Ball
  Cc: Tim Kryger, devicetree, linux-arm-kernel, linux-kernel,
	linux-mmc, patches

Rather than declaring the frequency of the external clock, specify the
label of the clock such that the driver may determine the frequency on
its own.

Signed-off-by: Tim Kryger <tim.kryger@linaro.org>
Reviewed-by: Markus Mayer <markus.mayer@linaro.org>
Reviewed-by: Matt Porter <matt.porter@linaro.org>
---
 arch/arm/boot/dts/bcm11351.dtsi | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/bcm11351.dtsi b/arch/arm/boot/dts/bcm11351.dtsi
index c6464fb..ce65367 100644
--- a/arch/arm/boot/dts/bcm11351.dtsi
+++ b/arch/arm/boot/dts/bcm11351.dtsi
@@ -43,7 +43,7 @@
 		compatible = "brcm,bcm11351-dw-apb-uart", "snps,dw-apb-uart";
 		status = "disabled";
 		reg = <0x3e000000 0x1000>;
-		clock-frequency = <13000000>;
+		clocks = <&uartb_clk>;
 		interrupts = <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>;
 		reg-shift = <2>;
 		reg-io-width = <4>;
@@ -53,7 +53,7 @@
 		compatible = "brcm,bcm11351-dw-apb-uart", "snps,dw-apb-uart";
 		status = "disabled";
 		reg = <0x3e001000 0x1000>;
-		clock-frequency = <13000000>;
+		clocks = <&uartb2_clk>;
 		interrupts = <GIC_SPI 66 IRQ_TYPE_LEVEL_HIGH>;
 		reg-shift = <2>;
 		reg-io-width = <4>;
@@ -63,7 +63,7 @@
 		compatible = "brcm,bcm11351-dw-apb-uart", "snps,dw-apb-uart";
 		status = "disabled";
 		reg = <0x3e002000 0x1000>;
-		clock-frequency = <13000000>;
+		clocks = <&uartb3_clk>;
 		interrupts = <GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>;
 		reg-shift = <2>;
 		reg-io-width = <4>;
@@ -73,7 +73,7 @@
 		compatible = "brcm,bcm11351-dw-apb-uart", "snps,dw-apb-uart";
 		status = "disabled";
 		reg = <0x3e003000 0x1000>;
-		clock-frequency = <13000000>;
+		clocks = <&uartb4_clk>;
 		interrupts = <GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>;
 		reg-shift = <2>;
 		reg-io-width = <4>;
-- 
1.8.0.1



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [RESEND PATCH v2 3/6] ARM: dts: Specify clocks for SDHCIs on bcm11351
  2013-10-16 21:47 [RESEND PATCH v2 0/6] Update Kona drivers to use clocks Tim Kryger
  2013-10-16 21:47 ` [RESEND PATCH v2 1/6] ARM: dts: Declare clocks as fixed on bcm11351 Tim Kryger
  2013-10-16 21:47 ` [RESEND PATCH v2 2/6] ARM: dts: Specify clocks for UARTs " Tim Kryger
@ 2013-10-16 21:47 ` Tim Kryger
  2013-10-16 21:47 ` [RESEND PATCH v2 4/6] clocksource: kona: Add basic use of external clock Tim Kryger
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 22+ messages in thread
From: Tim Kryger @ 2013-10-16 21:47 UTC (permalink / raw)
  To: Christian Daudt, Rob Herring, Pawel Moll, Mark Rutland,
	Stephen Warren, Ian Campbell, Daniel Lezcano, Thomas Gleixner,
	Chris Ball
  Cc: Tim Kryger, devicetree, linux-arm-kernel, linux-kernel,
	linux-mmc, patches

Specify the external clock label in each SDHCI node.

Signed-off-by: Tim Kryger <tim.kryger@linaro.org>
Reviewed-by: Markus Mayer <markus.mayer@linaro.org>
Reviewed-by: Matt Porter <matt.porter@linaro.org>
---
 arch/arm/boot/dts/bcm11351.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/bcm11351.dtsi b/arch/arm/boot/dts/bcm11351.dtsi
index ce65367..193659c 100644
--- a/arch/arm/boot/dts/bcm11351.dtsi
+++ b/arch/arm/boot/dts/bcm11351.dtsi
@@ -101,6 +101,7 @@
 	sdio1: sdio@3f180000 {
 		compatible = "brcm,kona-sdhci";
 		reg = <0x3f180000 0x10000>;
+		clocks = <&sdio1_clk>;
 		interrupts = <0x0 77 0x4>;
 		status = "disabled";
 	};
@@ -108,6 +109,7 @@
 	sdio2: sdio@3f190000 {
 		compatible = "brcm,kona-sdhci";
 		reg = <0x3f190000 0x10000>;
+		clocks = <&sdio2_clk>;
 		interrupts = <0x0 76 0x4>;
 		status = "disabled";
 	};
@@ -115,6 +117,7 @@
 	sdio3: sdio@3f1a0000 {
 		compatible = "brcm,kona-sdhci";
 		reg = <0x3f1a0000 0x10000>;
+		clocks = <&sdio3_clk>;
 		interrupts = <0x0 74 0x4>;
 		status = "disabled";
 	};
@@ -122,6 +125,7 @@
 	sdio4: sdio@3f1b0000 {
 		compatible = "brcm,kona-sdhci";
 		reg = <0x3f1b0000 0x10000>;
+		clocks = <&sdio4_clk>;
 		interrupts = <0x0 73 0x4>;
 		status = "disabled";
 	};
-- 
1.8.0.1



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [RESEND PATCH v2 4/6] clocksource: kona: Add basic use of external clock
  2013-10-16 21:47 [RESEND PATCH v2 0/6] Update Kona drivers to use clocks Tim Kryger
                   ` (2 preceding siblings ...)
  2013-10-16 21:47 ` [RESEND PATCH v2 3/6] ARM: dts: Specify clocks for SDHCIs " Tim Kryger
@ 2013-10-16 21:47 ` Tim Kryger
  2013-10-17 14:05   ` Mark Rutland
  2013-10-16 21:47 ` [RESEND PATCH v2 5/6] ARM: dts: Specify clocks for timer on bcm11351 Tim Kryger
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 22+ messages in thread
From: Tim Kryger @ 2013-10-16 21:47 UTC (permalink / raw)
  To: Christian Daudt, Rob Herring, Pawel Moll, Mark Rutland,
	Stephen Warren, Ian Campbell, Daniel Lezcano, Thomas Gleixner,
	Chris Ball
  Cc: Tim Kryger, devicetree, linux-arm-kernel, linux-kernel,
	linux-mmc, patches

When an clock handle is specified in the device tree, enable it and use
it to determine the external clock frequency.

Signed-off-by: Tim Kryger <tim.kryger@linaro.org>
Reviewed-by: Markus Mayer <markus.mayer@linaro.org>
Reviewed-by: Matt Porter <matt.porter@linaro.org>
---
 drivers/clocksource/bcm_kona_timer.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/clocksource/bcm_kona_timer.c b/drivers/clocksource/bcm_kona_timer.c
index 0d7d8c3..fd11f96 100644
--- a/drivers/clocksource/bcm_kona_timer.c
+++ b/drivers/clocksource/bcm_kona_timer.c
@@ -17,6 +17,7 @@
 #include <linux/jiffies.h>
 #include <linux/clockchips.h>
 #include <linux/types.h>
+#include <linux/clk.h>
 
 #include <linux/io.h>
 #include <asm/mach/time.h>
@@ -107,11 +108,18 @@ static const struct of_device_id bcm_timer_ids[] __initconst = {
 static void __init kona_timers_init(struct device_node *node)
 {
 	u32 freq;
+	struct clk *external_clk;
 
-	if (!of_property_read_u32(node, "clock-frequency", &freq))
+	external_clk = of_clk_get_by_name(node, NULL);
+
+	if (!IS_ERR(external_clk)) {
+		arch_timer_rate = clk_get_rate(external_clk);
+		clk_prepare_enable(external_clk);
+	} else if (!of_property_read_u32(node, "clock-frequency", &freq)) {
 		arch_timer_rate = freq;
-	else
-		panic("clock-frequency not set in the .dts file");
+	} else {
+		panic("neither clock-frequency or clocks handle in .dts file");
+	}
 
 	/* Setup IRQ numbers */
 	timers.tmr_irq = irq_of_parse_and_map(node, 0);
-- 
1.8.0.1



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [RESEND PATCH v2 5/6] ARM: dts: Specify clocks for timer on bcm11351
  2013-10-16 21:47 [RESEND PATCH v2 0/6] Update Kona drivers to use clocks Tim Kryger
                   ` (3 preceding siblings ...)
  2013-10-16 21:47 ` [RESEND PATCH v2 4/6] clocksource: kona: Add basic use of external clock Tim Kryger
@ 2013-10-16 21:47 ` Tim Kryger
  2013-10-17 14:06   ` Mark Rutland
  2013-10-16 21:47 ` [RESEND PATCH v2 6/6] mmc: sdhci-bcm-kona: Add basic use of clocks Tim Kryger
  2013-10-17 13:47 ` [RESEND PATCH v2 0/6] Update Kona drivers to use clocks Mark Rutland
  6 siblings, 1 reply; 22+ messages in thread
From: Tim Kryger @ 2013-10-16 21:47 UTC (permalink / raw)
  To: Christian Daudt, Rob Herring, Pawel Moll, Mark Rutland,
	Stephen Warren, Ian Campbell, Daniel Lezcano, Thomas Gleixner,
	Chris Ball
  Cc: Tim Kryger, devicetree, linux-arm-kernel, linux-kernel,
	linux-mmc, patches

Specify the external clock label in the timer node.

Signed-off-by: Tim Kryger <tim.kryger@linaro.org>
Reviewed-by: Markus Mayer <markus.mayer@linaro.org>
Reviewed-by: Matt Porter <matt.porter@linaro.org>
---
 arch/arm/boot/dts/bcm11351.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/bcm11351.dtsi b/arch/arm/boot/dts/bcm11351.dtsi
index 193659c..39c1395 100644
--- a/arch/arm/boot/dts/bcm11351.dtsi
+++ b/arch/arm/boot/dts/bcm11351.dtsi
@@ -95,7 +95,7 @@
 		compatible = "brcm,kona-timer";
 		reg = <0x35006000 0x1000>;
 		interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
-		clock-frequency = <32768>;
+		clocks = <&hub_timer_clk>;
 	};
 
 	sdio1: sdio@3f180000 {
-- 
1.8.0.1



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [RESEND PATCH v2 6/6] mmc: sdhci-bcm-kona: Add basic use of clocks
  2013-10-16 21:47 [RESEND PATCH v2 0/6] Update Kona drivers to use clocks Tim Kryger
                   ` (4 preceding siblings ...)
  2013-10-16 21:47 ` [RESEND PATCH v2 5/6] ARM: dts: Specify clocks for timer on bcm11351 Tim Kryger
@ 2013-10-16 21:47 ` Tim Kryger
  2013-10-17 14:13   ` Mark Rutland
  2013-10-17 13:47 ` [RESEND PATCH v2 0/6] Update Kona drivers to use clocks Mark Rutland
  6 siblings, 1 reply; 22+ messages in thread
From: Tim Kryger @ 2013-10-16 21:47 UTC (permalink / raw)
  To: Christian Daudt, Rob Herring, Pawel Moll, Mark Rutland,
	Stephen Warren, Ian Campbell, Daniel Lezcano, Thomas Gleixner,
	Chris Ball
  Cc: Tim Kryger, devicetree, linux-arm-kernel, linux-kernel,
	linux-mmc, patches

Enable the external clock needed by the host controller during the
probe and disable it during the remove.

Signed-off-by: Tim Kryger <tim.kryger@linaro.org>
Reviewed-by: Markus Mayer <markus.mayer@linaro.org>
Reviewed-by: Matt Porter <matt.porter@linaro.org>
---
 drivers/mmc/host/sdhci-bcm-kona.c | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci-bcm-kona.c b/drivers/mmc/host/sdhci-bcm-kona.c
index 85472d3..91db099 100644
--- a/drivers/mmc/host/sdhci-bcm-kona.c
+++ b/drivers/mmc/host/sdhci-bcm-kona.c
@@ -54,6 +54,7 @@
 
 struct sdhci_bcm_kona_dev {
 	struct mutex	write_lock; /* protect back to back writes */
+	struct clk	*external_clk;
 };
 
 
@@ -252,11 +253,29 @@ static int sdhci_bcm_kona_probe(struct platform_device *pdev)
 	mmc_of_parse(host->mmc);
 
 	if (!host->mmc->f_max) {
-		dev_err(&pdev->dev, "Missing max-freq for SDHCI cfg\n");
+		dev_err(dev, "Missing max-freq for SDHCI cfg\n");
 		ret = -ENXIO;
 		goto err_pltfm_free;
 	}
 
+	/* Get and enable the external clock */
+	kona_dev->external_clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(kona_dev->external_clk)) {
+		dev_err(dev, "Failed to get external clock\n");
+		ret = PTR_ERR(kona_dev->external_clk);
+		goto err_pltfm_free;
+	}
+
+	if (clk_set_rate(kona_dev->external_clk, host->mmc->f_max) != 0) {
+		dev_err(dev, "Failed to set rate external clock\n");
+		goto err_pltfm_free;
+	}
+
+	if (clk_prepare_enable(kona_dev->external_clk) != 0) {
+		dev_err(dev, "Failed to enable external clock\n");
+		goto err_pltfm_free;
+	}
+
 	dev_dbg(dev, "non-removable=%c\n",
 		(host->mmc->caps & MMC_CAP_NONREMOVABLE) ? 'Y' : 'N');
 	dev_dbg(dev, "cd_gpio %c, wp_gpio %c\n",
@@ -271,7 +290,7 @@ static int sdhci_bcm_kona_probe(struct platform_device *pdev)
 
 	ret = sdhci_bcm_kona_sd_reset(host);
 	if (ret)
-		goto err_pltfm_free;
+		goto err_clk_disable;
 
 	sdhci_bcm_kona_sd_init(host);
 
@@ -307,6 +326,9 @@ err_remove_host:
 err_reset:
 	sdhci_bcm_kona_sd_reset(host);
 
+err_clk_disable:
+	clk_disable_unprepare(kona_dev->external_clk);
+
 err_pltfm_free:
 	sdhci_pltfm_free(pdev);
 
@@ -317,6 +339,8 @@ err_pltfm_free:
 static int __exit sdhci_bcm_kona_remove(struct platform_device *pdev)
 {
 	struct sdhci_host *host = platform_get_drvdata(pdev);
+	struct sdhci_pltfm_host *pltfm_priv = sdhci_priv(host);
+	struct sdhci_bcm_kona_dev *kona_dev = sdhci_pltfm_priv(pltfm_priv);
 	int dead;
 	u32 scratch;
 
@@ -326,6 +350,8 @@ static int __exit sdhci_bcm_kona_remove(struct platform_device *pdev)
 		dead = 1;
 	sdhci_remove_host(host, dead);
 
+	clk_disable_unprepare(kona_dev->external_clk);
+
 	sdhci_free_host(host);
 
 	return 0;
-- 
1.8.0.1



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [RESEND PATCH v2 2/6] ARM: dts: Specify clocks for UARTs on bcm11351
  2013-10-16 21:47 ` [RESEND PATCH v2 2/6] ARM: dts: Specify clocks for UARTs " Tim Kryger
@ 2013-10-17  5:41   ` Christian Daudt
  2013-10-17  6:14     ` Sebastian Hesselbarth
  2013-10-17 13:56   ` Mark Rutland
  1 sibling, 1 reply; 22+ messages in thread
From: Christian Daudt @ 2013-10-17  5:41 UTC (permalink / raw)
  To: Tim Kryger, Matt Porter, Sebastian Hesselbarth
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
	Ian Campbell, Daniel Lezcano, Thomas Gleixner, Chris Ball,
	devicetree, linux-arm-kernel, linux-kernel, linux-mmc,
	Linaro Patches

On Wed, Oct 16, 2013 at 2:47 PM, Tim Kryger <tim.kryger@linaro.org> wrote:
> Rather than declaring the frequency of the external clock, specify the
> label of the clock such that the driver may determine the frequency on
> its own.
>
> Signed-off-by: Tim Kryger <tim.kryger@linaro.org>
> Reviewed-by: Markus Mayer <markus.mayer@linaro.org>
> Reviewed-by: Matt Porter <matt.porter@linaro.org>
> ---
>  arch/arm/boot/dts/bcm11351.dtsi | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/boot/dts/bcm11351.dtsi b/arch/arm/boot/dts/bcm11351.dtsi
> index c6464fb..ce65367 100644
> --- a/arch/arm/boot/dts/bcm11351.dtsi
> +++ b/arch/arm/boot/dts/bcm11351.dtsi
> @@ -43,7 +43,7 @@
>                 compatible = "brcm,bcm11351-dw-apb-uart", "snps,dw-apb-uart";
>                 status = "disabled";
>                 reg = <0x3e000000 0x1000>;
> -               clock-frequency = <13000000>;
> +               clocks = <&uartb_clk>;
>                 interrupts = <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>;
>                 reg-shift = <2>;
>                 reg-io-width = <4>;

Hi Sebastian,
 this patch series (and a subsequent one from Tim) both rely on your
"ARM: provide common arch init for DT clocks" patchset in order to
work. Will that patchset be in 3.13 ? I don't want to pull the dt mods
unless they are as they break the boot without them.

 Thanks,
   csd

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RESEND PATCH v2 2/6] ARM: dts: Specify clocks for UARTs on bcm11351
  2013-10-17  5:41   ` Christian Daudt
@ 2013-10-17  6:14     ` Sebastian Hesselbarth
  0 siblings, 0 replies; 22+ messages in thread
From: Sebastian Hesselbarth @ 2013-10-17  6:14 UTC (permalink / raw)
  To: Christian Daudt
  Cc: Tim Kryger, Matt Porter, Rob Herring, Pawel Moll, Mark Rutland,
	Stephen Warren, Ian Campbell, Daniel Lezcano, Thomas Gleixner,
	Chris Ball, devicetree, linux-arm-kernel, linux-kernel,
	linux-mmc, Linaro Patches

On 10/17/2013 07:41 AM, Christian Daudt wrote:
> On Wed, Oct 16, 2013 at 2:47 PM, Tim Kryger <tim.kryger@linaro.org> wrote:
>> Rather than declaring the frequency of the external clock, specify the
>> label of the clock such that the driver may determine the frequency on
>> its own.
>>
>> Signed-off-by: Tim Kryger <tim.kryger@linaro.org>
>> Reviewed-by: Markus Mayer <markus.mayer@linaro.org>
>> Reviewed-by: Matt Porter <matt.porter@linaro.org>
>> ---
>>   arch/arm/boot/dts/bcm11351.dtsi | 8 ++++----
>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/bcm11351.dtsi b/arch/arm/boot/dts/bcm11351.dtsi
>> index c6464fb..ce65367 100644
>> --- a/arch/arm/boot/dts/bcm11351.dtsi
>> +++ b/arch/arm/boot/dts/bcm11351.dtsi
>> @@ -43,7 +43,7 @@
>>                  compatible = "brcm,bcm11351-dw-apb-uart", "snps,dw-apb-uart";
>>                  status = "disabled";
>>                  reg = <0x3e000000 0x1000>;
>> -               clock-frequency = <13000000>;
>> +               clocks = <&uartb_clk>;
>>                  interrupts = <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>;
>>                  reg-shift = <2>;
>>                  reg-io-width = <4>;
>
> Hi Sebastian,
>   this patch series (and a subsequent one from Tim) both rely on your
> "ARM: provide common arch init for DT clocks" patchset in order to
> work. Will that patchset be in 3.13 ? I don't want to pull the dt mods
> unless they are as they break the boot without them.

Christian,

here is the pull request [1], pull [2], and stable commitment for the
branch [3]. You can merge that branch too and name the dependency when
you send your PR, arm-soc maintainers will sort it out.

Besides a small section mismatch fixup for nomadik that Olof requested,
I guess it is in.

Sebastian

[1] http://www.spinics.net/lists/arm-kernel/msg276175.html
[2] http://www.spinics.net/lists/arm-kernel/msg277816.html
[3] http://www.spinics.net/lists/arm-kernel/msg279736.html


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RESEND PATCH v2 0/6] Update Kona drivers to use clocks
  2013-10-16 21:47 [RESEND PATCH v2 0/6] Update Kona drivers to use clocks Tim Kryger
                   ` (5 preceding siblings ...)
  2013-10-16 21:47 ` [RESEND PATCH v2 6/6] mmc: sdhci-bcm-kona: Add basic use of clocks Tim Kryger
@ 2013-10-17 13:47 ` Mark Rutland
  6 siblings, 0 replies; 22+ messages in thread
From: Mark Rutland @ 2013-10-17 13:47 UTC (permalink / raw)
  To: Tim Kryger
  Cc: Christian Daudt, rob.herring, Pawel Moll, Stephen Warren,
	Ian Campbell, Daniel Lezcano, Thomas Gleixner, Chris Ball,
	devicetree, linux-arm-kernel, linux-kernel, linux-mmc, patches

On Wed, Oct 16, 2013 at 10:47:04PM +0100, Tim Kryger wrote:
> This series declares the clocks present on bcm11351 (aka bcm281xx) SoCs
> to be fixed at the rates configured by development bootloaders and then
> updates the Kona drivers to make use of clock calls where appropriate.
> 
> This patch series depends on the following three commits being present:
> 
> https://lkml.org/lkml/2013/9/18/409
> https://lkml.org/lkml/2013/9/20/305
> http://www.spinics.net/lists/arm-kernel/msg274973.html
> 
> Changes in v2:
>   - Renamed bsc4_clk to pmu_bsc_clk
> 
> Tim Kryger (6):
>   ARM: dts: Declare clocks as fixed on bcm11351
>   ARM: dts: Specify clocks for UARTs on bcm11351
>   ARM: dts: Specify clocks for SDHCIs on bcm11351
>   clocksource: kona: Add basic use of external clock
>   ARM: dts: Specify clocks for timer on bcm11351
>   mmc: sdhci-bcm-kona: Add basic use of clocks
> 
>  arch/arm/boot/dts/bcm11351.dtsi      | 111 +++++++++++++++++++++++++++++++++--
>  drivers/clocksource/bcm_kona_timer.c |  14 ++++-
>  drivers/mmc/host/sdhci-bcm-kona.c    |  30 +++++++++-

This requires a description in the binding documents, so we can figure
out which clocks we need to describe, and how to describe them.

Thanks,
Mark.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RESEND PATCH v2 2/6] ARM: dts: Specify clocks for UARTs on bcm11351
  2013-10-16 21:47 ` [RESEND PATCH v2 2/6] ARM: dts: Specify clocks for UARTs " Tim Kryger
  2013-10-17  5:41   ` Christian Daudt
@ 2013-10-17 13:56   ` Mark Rutland
  2013-10-21 18:56     ` Tim Kryger
  1 sibling, 1 reply; 22+ messages in thread
From: Mark Rutland @ 2013-10-17 13:56 UTC (permalink / raw)
  To: Tim Kryger
  Cc: Christian Daudt, rob.herring, Pawel Moll, Stephen Warren,
	Ian Campbell, Daniel Lezcano, Thomas Gleixner, Chris Ball,
	devicetree, linux-arm-kernel, linux-kernel, linux-mmc, patches

On Wed, Oct 16, 2013 at 10:47:06PM +0100, Tim Kryger wrote:
> Rather than declaring the frequency of the external clock, specify the
> label of the clock such that the driver may determine the frequency on
> its own.

Nit: we're not specifying the label of the clock. Clocks are represented
py a phand+args pair, and the important part is that we're specifying
the clock itself.

How about something like:

---->8----
Currently the rate of the external clock input to "snps,dw-apb-uart"
devices is described by a clock-frequency property rather than by
reference to the clock itself.

This patch changes the "snps,dw-apb-uart" entries in bcm11351.dtsi to
refer to the parent clock directly, following the common clock bindings.
---->8----

Also, this should probably be moved after the driver change, so as to
not be unbootable temporarily.

Thanks,
Mark.

> 
> Signed-off-by: Tim Kryger <tim.kryger@linaro.org>
> Reviewed-by: Markus Mayer <markus.mayer@linaro.org>
> Reviewed-by: Matt Porter <matt.porter@linaro.org>
> ---
>  arch/arm/boot/dts/bcm11351.dtsi | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/bcm11351.dtsi b/arch/arm/boot/dts/bcm11351.dtsi
> index c6464fb..ce65367 100644
> --- a/arch/arm/boot/dts/bcm11351.dtsi
> +++ b/arch/arm/boot/dts/bcm11351.dtsi
> @@ -43,7 +43,7 @@
>  		compatible = "brcm,bcm11351-dw-apb-uart", "snps,dw-apb-uart";
>  		status = "disabled";
>  		reg = <0x3e000000 0x1000>;
> -		clock-frequency = <13000000>;
> +		clocks = <&uartb_clk>;
>  		interrupts = <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>;
>  		reg-shift = <2>;
>  		reg-io-width = <4>;
> @@ -53,7 +53,7 @@
>  		compatible = "brcm,bcm11351-dw-apb-uart", "snps,dw-apb-uart";
>  		status = "disabled";
>  		reg = <0x3e001000 0x1000>;
> -		clock-frequency = <13000000>;
> +		clocks = <&uartb2_clk>;
>  		interrupts = <GIC_SPI 66 IRQ_TYPE_LEVEL_HIGH>;
>  		reg-shift = <2>;
>  		reg-io-width = <4>;
> @@ -63,7 +63,7 @@
>  		compatible = "brcm,bcm11351-dw-apb-uart", "snps,dw-apb-uart";
>  		status = "disabled";
>  		reg = <0x3e002000 0x1000>;
> -		clock-frequency = <13000000>;
> +		clocks = <&uartb3_clk>;
>  		interrupts = <GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>;
>  		reg-shift = <2>;
>  		reg-io-width = <4>;
> @@ -73,7 +73,7 @@
>  		compatible = "brcm,bcm11351-dw-apb-uart", "snps,dw-apb-uart";
>  		status = "disabled";
>  		reg = <0x3e003000 0x1000>;
> -		clock-frequency = <13000000>;
> +		clocks = <&uartb4_clk>;
>  		interrupts = <GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>;
>  		reg-shift = <2>;
>  		reg-io-width = <4>;
> -- 
> 1.8.0.1
> 
> 
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RESEND PATCH v2 4/6] clocksource: kona: Add basic use of external clock
  2013-10-16 21:47 ` [RESEND PATCH v2 4/6] clocksource: kona: Add basic use of external clock Tim Kryger
@ 2013-10-17 14:05   ` Mark Rutland
  2013-10-21 21:07     ` Tim Kryger
  0 siblings, 1 reply; 22+ messages in thread
From: Mark Rutland @ 2013-10-17 14:05 UTC (permalink / raw)
  To: Tim Kryger
  Cc: Christian Daudt, rob.herring, Pawel Moll, Stephen Warren,
	Ian Campbell, Daniel Lezcano, Thomas Gleixner, Chris Ball,
	devicetree, linux-arm-kernel, linux-kernel, linux-mmc, patches

On Wed, Oct 16, 2013 at 10:47:08PM +0100, Tim Kryger wrote:
> When an clock handle is specified in the device tree, enable it and use
> it to determine the external clock frequency.

I'd drop handle here and just say "When a clock is specified".

This will need a binding document update.

> 
> Signed-off-by: Tim Kryger <tim.kryger@linaro.org>
> Reviewed-by: Markus Mayer <markus.mayer@linaro.org>
> Reviewed-by: Matt Porter <matt.porter@linaro.org>
> ---
>  drivers/clocksource/bcm_kona_timer.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clocksource/bcm_kona_timer.c b/drivers/clocksource/bcm_kona_timer.c
> index 0d7d8c3..fd11f96 100644
> --- a/drivers/clocksource/bcm_kona_timer.c
> +++ b/drivers/clocksource/bcm_kona_timer.c
> @@ -17,6 +17,7 @@
>  #include <linux/jiffies.h>
>  #include <linux/clockchips.h>
>  #include <linux/types.h>
> +#include <linux/clk.h>
>  
>  #include <linux/io.h>
>  #include <asm/mach/time.h>
> @@ -107,11 +108,18 @@ static const struct of_device_id bcm_timer_ids[] __initconst = {
>  static void __init kona_timers_init(struct device_node *node)
>  {
>  	u32 freq;
> +	struct clk *external_clk;
>  
> -	if (!of_property_read_u32(node, "clock-frequency", &freq))
> +	external_clk = of_clk_get_by_name(node, NULL);

Is there only a single external clock input to the kona timer, or is
only one relevant here?

Are there any other inputs currently not modelled (e.g. regulators)?

> +
> +	if (!IS_ERR(external_clk)) {
> +		arch_timer_rate = clk_get_rate(external_clk);
> +		clk_prepare_enable(external_clk);
> +	} else if (!of_property_read_u32(node, "clock-frequency", &freq)) {
>  		arch_timer_rate = freq;
> -	else
> -		panic("clock-frequency not set in the .dts file");
> +	} else {
> +		panic("neither clock-frequency or clocks handle in .dts file");

Nit: it's not a handle, it's a phandle+args pair.

Why not just "Unable to determine clock frequency"?

Do we need to panic here? Might a system have other clocks it could use
to continue?

Thanks,
Mark.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RESEND PATCH v2 5/6] ARM: dts: Specify clocks for timer on bcm11351
  2013-10-16 21:47 ` [RESEND PATCH v2 5/6] ARM: dts: Specify clocks for timer on bcm11351 Tim Kryger
@ 2013-10-17 14:06   ` Mark Rutland
  2013-10-18 20:25     ` Tim Kryger
  0 siblings, 1 reply; 22+ messages in thread
From: Mark Rutland @ 2013-10-17 14:06 UTC (permalink / raw)
  To: Tim Kryger
  Cc: Christian Daudt, rob.herring, Pawel Moll, Stephen Warren,
	Ian Campbell, Daniel Lezcano, Thomas Gleixner, Chris Ball,
	devicetree, linux-arm-kernel, linux-kernel, linux-mmc, patches

> diff --git a/arch/arm/boot/dts/bcm11351.dtsi b/arch/arm/boot/dts/bcm11351.dtsi
> index 193659c..39c1395 100644
> --- a/arch/arm/boot/dts/bcm11351.dtsi
> +++ b/arch/arm/boot/dts/bcm11351.dtsi
> @@ -95,7 +95,7 @@
>  		compatible = "brcm,kona-timer";
>  		reg = <0x35006000 0x1000>;
>  		interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
> -		clock-frequency = <32768>;
> +		clocks = <&hub_timer_clk>;

This should probably be after the code change that enables support for a
clocks property.

Thanks,
Mark.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RESEND PATCH v2 6/6] mmc: sdhci-bcm-kona: Add basic use of clocks
  2013-10-16 21:47 ` [RESEND PATCH v2 6/6] mmc: sdhci-bcm-kona: Add basic use of clocks Tim Kryger
@ 2013-10-17 14:13   ` Mark Rutland
  2013-10-18 13:22     ` Matt Porter
  2013-10-21 23:13     ` Tim Kryger
  0 siblings, 2 replies; 22+ messages in thread
From: Mark Rutland @ 2013-10-17 14:13 UTC (permalink / raw)
  To: Tim Kryger
  Cc: Christian Daudt, rob.herring, Pawel Moll, Stephen Warren,
	Ian Campbell, Daniel Lezcano, Thomas Gleixner, Chris Ball,
	devicetree, linux-arm-kernel, linux-kernel, linux-mmc, patches

On Wed, Oct 16, 2013 at 10:47:10PM +0100, Tim Kryger wrote:
> Enable the external clock needed by the host controller during the
> probe and disable it during the remove.

This requires a biding document update.

I note that the binding is already incomplete (it does not describe the
interrupts or reg).

> 
> Signed-off-by: Tim Kryger <tim.kryger@linaro.org>
> Reviewed-by: Markus Mayer <markus.mayer@linaro.org>
> Reviewed-by: Matt Porter <matt.porter@linaro.org>
> ---
>  drivers/mmc/host/sdhci-bcm-kona.c | 30 ++++++++++++++++++++++++++++--
>  1 file changed, 28 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-bcm-kona.c b/drivers/mmc/host/sdhci-bcm-kona.c
> index 85472d3..91db099 100644
> --- a/drivers/mmc/host/sdhci-bcm-kona.c
> +++ b/drivers/mmc/host/sdhci-bcm-kona.c
> @@ -54,6 +54,7 @@
>  
>  struct sdhci_bcm_kona_dev {
>  	struct mutex	write_lock; /* protect back to back writes */
> +	struct clk	*external_clk;

Is this the only clock input the unit has?

Are there any other external resources the device needs (e.g. gpios,
regulators)?

>  };
>  
>  
> @@ -252,11 +253,29 @@ static int sdhci_bcm_kona_probe(struct platform_device *pdev)
>  	mmc_of_parse(host->mmc);
>  
>  	if (!host->mmc->f_max) {
> -		dev_err(&pdev->dev, "Missing max-freq for SDHCI cfg\n");
> +		dev_err(dev, "Missing max-freq for SDHCI cfg\n");

This seems like an unrelated change.

>  		ret = -ENXIO;
>  		goto err_pltfm_free;
>  	}
>  
> +	/* Get and enable the external clock */
> +	kona_dev->external_clk = devm_clk_get(dev, NULL);
> +	if (IS_ERR(kona_dev->external_clk)) {
> +		dev_err(dev, "Failed to get external clock\n");
> +		ret = PTR_ERR(kona_dev->external_clk);
> +		goto err_pltfm_free;

This seems like a pretty clear breakage of existing DTBs.

Why?

Thanks,
Mark.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RESEND PATCH v2 6/6] mmc: sdhci-bcm-kona: Add basic use of clocks
  2013-10-17 14:13   ` Mark Rutland
@ 2013-10-18 13:22     ` Matt Porter
  2013-10-21 23:13     ` Tim Kryger
  1 sibling, 0 replies; 22+ messages in thread
From: Matt Porter @ 2013-10-18 13:22 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Tim Kryger, devicetree, Christian Daudt, Pawel Moll,
	Ian Campbell, Daniel Lezcano, linux-mmc, linux-kernel,
	rob.herring, patches, Stephen Warren, Thomas Gleixner,
	Chris Ball, linux-arm-kernel

On Thu, Oct 17, 2013 at 03:13:09PM +0100, Mark Rutland wrote:
> On Wed, Oct 16, 2013 at 10:47:10PM +0100, Tim Kryger wrote:
> > +	/* Get and enable the external clock */
> > +	kona_dev->external_clk = devm_clk_get(dev, NULL);
> > +	if (IS_ERR(kona_dev->external_clk)) {
> > +		dev_err(dev, "Failed to get external clock\n");
> > +		ret = PTR_ERR(kona_dev->external_clk);
> > +		goto err_pltfm_free;
> 
> This seems like a pretty clear breakage of existing DTBs.
> 
> Why?

Normal kernel incremental development. The notion of breaking DTBs is
pretty much incompatible with the long-standing tradition of developing
kernel support in an incremental fashion.

In this case, the bootloader available on these boards has had clocks
and regulators enabled for all the drivers currently upstream. This
allowed development of the sdhci driver (and others) independent of having
the common clk driver and PMU/regulator drivers ready. That's been especially
important given the flux that the common clk framework is in with regards
to bindings.

Are you suggesting that the required clock needs to be made logically
optional in the code to retain backward compatbility?

-Matt

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RESEND PATCH v2 5/6] ARM: dts: Specify clocks for timer on bcm11351
  2013-10-17 14:06   ` Mark Rutland
@ 2013-10-18 20:25     ` Tim Kryger
  0 siblings, 0 replies; 22+ messages in thread
From: Tim Kryger @ 2013-10-18 20:25 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Christian Daudt, rob.herring, Pawel Moll, Stephen Warren,
	Ian Campbell, Daniel Lezcano, Thomas Gleixner, Chris Ball,
	devicetree, linux-arm-kernel, linux-kernel, linux-mmc, patches

On Thu, Oct 17, 2013 at 7:06 AM, Mark Rutland <mark.rutland@arm.com> wrote:
>> diff --git a/arch/arm/boot/dts/bcm11351.dtsi b/arch/arm/boot/dts/bcm11351.dtsi
>> index 193659c..39c1395 100644
>> --- a/arch/arm/boot/dts/bcm11351.dtsi
>> +++ b/arch/arm/boot/dts/bcm11351.dtsi
>> @@ -95,7 +95,7 @@
>>               compatible = "brcm,kona-timer";
>>               reg = <0x35006000 0x1000>;
>>               interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
>> -             clock-frequency = <32768>;
>> +             clocks = <&hub_timer_clk>;
>
> This should probably be after the code change that enables support for a
> clocks property.
>
> Thanks,
> Mark.

This is already the case.

The driver change is patch 4 and this dts change is patch 5 of the series.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RESEND PATCH v2 2/6] ARM: dts: Specify clocks for UARTs on bcm11351
  2013-10-17 13:56   ` Mark Rutland
@ 2013-10-21 18:56     ` Tim Kryger
  0 siblings, 0 replies; 22+ messages in thread
From: Tim Kryger @ 2013-10-21 18:56 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Christian Daudt, rob.herring, Pawel Moll, Stephen Warren,
	Ian Campbell, Daniel Lezcano, Thomas Gleixner, Chris Ball,
	devicetree, linux-arm-kernel, linux-kernel, linux-mmc, patches

On Thu, Oct 17, 2013 at 6:56 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> On Wed, Oct 16, 2013 at 10:47:06PM +0100, Tim Kryger wrote:
>> Rather than declaring the frequency of the external clock, specify the
>> label of the clock such that the driver may determine the frequency on
>> its own.
>
> Nit: we're not specifying the label of the clock. Clocks are represented
> py a phand+args pair, and the important part is that we're specifying
> the clock itself.
>
> How about something like:
>
> ---->8----
> Currently the rate of the external clock input to "snps,dw-apb-uart"
> devices is described by a clock-frequency property rather than by
> reference to the clock itself.
>
> This patch changes the "snps,dw-apb-uart" entries in bcm11351.dtsi to
> refer to the parent clock directly, following the common clock bindings.
> ---->8----

Would this be acceptable?

The frequency property in "snps,dw-apb-uart" entries are no longer
required if the rate of the external clock can be determined using the
clk api (see e302cd9 serial: 8250_dw: add support for clk api).

This patch replaces the frequency property in the UART nodes of
bcm11351.dtsi with references to the relevant clocks following the
common clock binding.

>
> Also, this should probably be moved after the driver change, so as to
> not be unbootable temporarily.

The driver change was part of v3.10 so there is no issue here.

>
> Thanks,
> Mark.
>
>>
>> Signed-off-by: Tim Kryger <tim.kryger@linaro.org>
>> Reviewed-by: Markus Mayer <markus.mayer@linaro.org>
>> Reviewed-by: Matt Porter <matt.porter@linaro.org>
>> ---
>>  arch/arm/boot/dts/bcm11351.dtsi | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/bcm11351.dtsi b/arch/arm/boot/dts/bcm11351.dtsi
>> index c6464fb..ce65367 100644
>> --- a/arch/arm/boot/dts/bcm11351.dtsi
>> +++ b/arch/arm/boot/dts/bcm11351.dtsi
>> @@ -43,7 +43,7 @@
>>               compatible = "brcm,bcm11351-dw-apb-uart", "snps,dw-apb-uart";
>>               status = "disabled";
>>               reg = <0x3e000000 0x1000>;
>> -             clock-frequency = <13000000>;
>> +             clocks = <&uartb_clk>;
>>               interrupts = <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>;
>>               reg-shift = <2>;
>>               reg-io-width = <4>;
>> @@ -53,7 +53,7 @@
>>               compatible = "brcm,bcm11351-dw-apb-uart", "snps,dw-apb-uart";
>>               status = "disabled";
>>               reg = <0x3e001000 0x1000>;
>> -             clock-frequency = <13000000>;
>> +             clocks = <&uartb2_clk>;
>>               interrupts = <GIC_SPI 66 IRQ_TYPE_LEVEL_HIGH>;
>>               reg-shift = <2>;
>>               reg-io-width = <4>;
>> @@ -63,7 +63,7 @@
>>               compatible = "brcm,bcm11351-dw-apb-uart", "snps,dw-apb-uart";
>>               status = "disabled";
>>               reg = <0x3e002000 0x1000>;
>> -             clock-frequency = <13000000>;
>> +             clocks = <&uartb3_clk>;
>>               interrupts = <GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>;
>>               reg-shift = <2>;
>>               reg-io-width = <4>;
>> @@ -73,7 +73,7 @@
>>               compatible = "brcm,bcm11351-dw-apb-uart", "snps,dw-apb-uart";
>>               status = "disabled";
>>               reg = <0x3e003000 0x1000>;
>> -             clock-frequency = <13000000>;
>> +             clocks = <&uartb4_clk>;
>>               interrupts = <GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>;
>>               reg-shift = <2>;
>>               reg-io-width = <4>;
>> --
>> 1.8.0.1
>>
>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RESEND PATCH v2 4/6] clocksource: kona: Add basic use of external clock
  2013-10-17 14:05   ` Mark Rutland
@ 2013-10-21 21:07     ` Tim Kryger
  0 siblings, 0 replies; 22+ messages in thread
From: Tim Kryger @ 2013-10-21 21:07 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Christian Daudt, rob.herring, Pawel Moll, Stephen Warren,
	Ian Campbell, Daniel Lezcano, Thomas Gleixner, Chris Ball,
	devicetree, linux-arm-kernel, linux-kernel, linux-mmc, patches

On Thu, Oct 17, 2013 at 7:05 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> On Wed, Oct 16, 2013 at 10:47:08PM +0100, Tim Kryger wrote:
>> When an clock handle is specified in the device tree, enable it and use
>> it to determine the external clock frequency.
>
> I'd drop handle here and just say "When a clock is specified".
>
> This will need a binding document update.

Can you be more specific about what you are looking for here?

Shall I just say that the frequency property is now deprecated since
it actually belongs to the external clock whose association with a
kona timer instance might be declared in the DT using the common clock
bindings but could also be declared through other means?

>> Signed-off-by: Tim Kryger <tim.kryger@linaro.org>
>> Reviewed-by: Markus Mayer <markus.mayer@linaro.org>
>> Reviewed-by: Matt Porter <matt.porter@linaro.org>
>> ---
>>  drivers/clocksource/bcm_kona_timer.c | 14 +++++++++++---
>>  1 file changed, 11 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/clocksource/bcm_kona_timer.c b/drivers/clocksource/bcm_kona_timer.c
>> index 0d7d8c3..fd11f96 100644
>> --- a/drivers/clocksource/bcm_kona_timer.c
>> +++ b/drivers/clocksource/bcm_kona_timer.c
>> @@ -17,6 +17,7 @@
>>  #include <linux/jiffies.h>
>>  #include <linux/clockchips.h>
>>  #include <linux/types.h>
>> +#include <linux/clk.h>
>>
>>  #include <linux/io.h>
>>  #include <asm/mach/time.h>
>> @@ -107,11 +108,18 @@ static const struct of_device_id bcm_timer_ids[] __initconst = {
>>  static void __init kona_timers_init(struct device_node *node)
>>  {
>>       u32 freq;
>> +     struct clk *external_clk;
>>
>> -     if (!of_property_read_u32(node, "clock-frequency", &freq))
>> +     external_clk = of_clk_get_by_name(node, NULL);
>
> Is there only a single external clock input to the kona timer, or is
> only one relevant here?
>
> Are there any other inputs currently not modelled (e.g. regulators)?

The timer block relies upon a single clock and does not use any regulators.

>> +
>> +     if (!IS_ERR(external_clk)) {
>> +             arch_timer_rate = clk_get_rate(external_clk);
>> +             clk_prepare_enable(external_clk);
>> +     } else if (!of_property_read_u32(node, "clock-frequency", &freq)) {
>>               arch_timer_rate = freq;
>> -     else
>> -             panic("clock-frequency not set in the .dts file");
>> +     } else {
>> +             panic("neither clock-frequency or clocks handle in .dts file");
>
> Nit: it's not a handle, it's a phandle+args pair.
>
> Why not just "Unable to determine clock frequency"?

Sure that sounds fine.

>
> Do we need to panic here? Might a system have other clocks it could use
> to continue?

I completely agree that this driver could be improved.  However, this
is unrelated to the topic of this series so I will address it later in
a separate patch.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RESEND PATCH v2 6/6] mmc: sdhci-bcm-kona: Add basic use of clocks
  2013-10-17 14:13   ` Mark Rutland
  2013-10-18 13:22     ` Matt Porter
@ 2013-10-21 23:13     ` Tim Kryger
  1 sibling, 0 replies; 22+ messages in thread
From: Tim Kryger @ 2013-10-21 23:13 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Christian Daudt, rob.herring, Pawel Moll, Stephen Warren,
	Ian Campbell, Daniel Lezcano, Thomas Gleixner, Chris Ball,
	devicetree, linux-arm-kernel, linux-kernel, linux-mmc, patches

On Thu, Oct 17, 2013 at 7:13 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> On Wed, Oct 16, 2013 at 10:47:10PM +0100, Tim Kryger wrote:
>> Enable the external clock needed by the host controller during the
>> probe and disable it during the remove.
>
> This requires a biding document update.

The current best practice for declaring the association of a clock
with a consumer device is to embed the common clock bindings 'clocks'
property inside its node but this is not the only way it may be done.

Given that clk_get() still works for clocks found using string
matching, is it appropriate to mandate that all drivers that uses the
clk api mention the common clock bindings 'clocks' property in the
documentation for their device's binding?

> I note that the binding is already incomplete (it does not describe the
> interrupts or reg).

The current document reflects the standard for
Documentation/devicetree/bindings/mmc where normal properties are
described in mmc.txt and only the deviations described in vendor
specific files.

>>
>> Signed-off-by: Tim Kryger <tim.kryger@linaro.org>
>> Reviewed-by: Markus Mayer <markus.mayer@linaro.org>
>> Reviewed-by: Matt Porter <matt.porter@linaro.org>
>> ---
>>  drivers/mmc/host/sdhci-bcm-kona.c | 30 ++++++++++++++++++++++++++++--
>>  1 file changed, 28 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci-bcm-kona.c b/drivers/mmc/host/sdhci-bcm-kona.c
>> index 85472d3..91db099 100644
>> --- a/drivers/mmc/host/sdhci-bcm-kona.c
>> +++ b/drivers/mmc/host/sdhci-bcm-kona.c
>> @@ -54,6 +54,7 @@
>>
>>  struct sdhci_bcm_kona_dev {
>>       struct mutex    write_lock; /* protect back to back writes */
>> +     struct clk      *external_clk;
>
> Is this the only clock input the unit has?

The SDHCI block only receives one clock.

> Are there any other external resources the device needs (e.g. gpios,
> regulators)?

Yes but the cd-gpio and vmmc/vqmmc regulators are handled by the
common SDHCI driver.

>
>>  };
>>
>>
>> @@ -252,11 +253,29 @@ static int sdhci_bcm_kona_probe(struct platform_device *pdev)
>>       mmc_of_parse(host->mmc);
>>
>>       if (!host->mmc->f_max) {
>> -             dev_err(&pdev->dev, "Missing max-freq for SDHCI cfg\n");
>> +             dev_err(dev, "Missing max-freq for SDHCI cfg\n");
>
> This seems like an unrelated change.

Agreed.  I will remove this change.

>>               ret = -ENXIO;
>>               goto err_pltfm_free;
>>       }
>>
>> +     /* Get and enable the external clock */
>> +     kona_dev->external_clk = devm_clk_get(dev, NULL);
>> +     if (IS_ERR(kona_dev->external_clk)) {
>> +             dev_err(dev, "Failed to get external clock\n");
>> +             ret = PTR_ERR(kona_dev->external_clk);
>> +             goto err_pltfm_free;
>
> This seems like a pretty clear breakage of existing DTBs.
>
> Why?

The defconfig that builds this driver and DTBs currently sets
CONFIG_ARM_APPENDED_DTB=y so there isn't any actual breakage risk.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RESEND PATCH v2 1/6] ARM: dts: Declare clocks as fixed on bcm11351
  2013-10-16 21:47 ` [RESEND PATCH v2 1/6] ARM: dts: Declare clocks as fixed on bcm11351 Tim Kryger
@ 2013-11-08 10:53   ` Daniel Lezcano
  2013-11-08 19:42     ` Tim Kryger
  0 siblings, 1 reply; 22+ messages in thread
From: Daniel Lezcano @ 2013-11-08 10:53 UTC (permalink / raw)
  To: Tim Kryger, Christian Daudt, Rob Herring, Pawel Moll,
	Mark Rutland, Stephen Warren, Ian Campbell, Thomas Gleixner,
	Chris Ball
  Cc: devicetree, linux-arm-kernel, linux-kernel, linux-mmc, patches

On 10/16/2013 11:47 PM, Tim Kryger wrote:
> Declare clocks that are enabled and configured by bootloaders as fixed
> rate clocks in the DTS such that device drivers may use standard clock
> function calls.
>
> Signed-off-by: Tim Kryger <tim.kryger@linaro.org>
> Reviewed-by: Markus Mayer <markus.mayer@linaro.org>
> Reviewed-by: Matt Porter <matt.porter@linaro.org>
> ---

Hi Tim,

I was wondering what is the status of this patchset ?

The patchset touches different areas maintained by different people. 
Through what tree do you expect this patchset to be merged ??

Thanks
   -- Daniel

>   arch/arm/boot/dts/bcm11351.dtsi | 97 +++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 97 insertions(+)
>
> diff --git a/arch/arm/boot/dts/bcm11351.dtsi b/arch/arm/boot/dts/bcm11351.dtsi
> index 98e6bc0..c6464fb 100644
> --- a/arch/arm/boot/dts/bcm11351.dtsi
> +++ b/arch/arm/boot/dts/bcm11351.dtsi
> @@ -126,4 +126,101 @@
>   		status = "disabled";
>   	};
>
> +	clocks {
> +		bsc1_clk: bsc1 {
> +			compatible = "fixed-clock";
> +			clock-frequency = <13000000>;
> +			#clock-cells = <0>;
> +		};
> +
> +		bsc2_clk: bsc2 {
> +			compatible = "fixed-clock";
> +			clock-frequency = <13000000>;
> +			#clock-cells = <0>;
> +		};
> +
> +		bsc3_clk: bsc3 {
> +			compatible = "fixed-clock";
> +			clock-frequency = <13000000>;
> +			#clock-cells = <0>;
> +		};
> +
> +		pmu_bsc_clk: pmu_bsc {
> +			compatible = "fixed-clock";
> +			clock-frequency = <13000000>;
> +			#clock-cells = <0>;
> +		};
> +
> +		hub_timer_clk: hub_timer {
> +			compatible = "fixed-clock";
> +			clock-frequency = <32768>;
> +			#clock-cells = <0>;
> +		};
> +
> +		pwm_clk: pwm {
> +			compatible = "fixed-clock";
> +			clock-frequency = <26000000>;
> +			#clock-cells = <0>;
> +		};
> +
> +		sdio1_clk: sdio1 {
> +			compatible = "fixed-clock";
> +			clock-frequency = <48000000>;
> +			#clock-cells = <0>;
> +		};
> +
> +		sdio2_clk: sdio2 {
> +			compatible = "fixed-clock";
> +			clock-frequency = <48000000>;
> +			#clock-cells = <0>;
> +		};
> +
> +		sdio3_clk: sdio3 {
> +			compatible = "fixed-clock";
> +			clock-frequency = <48000000>;
> +			#clock-cells = <0>;
> +		};
> +
> +		sdio4_clk: sdio4 {
> +			compatible = "fixed-clock";
> +			clock-frequency = <48000000>;
> +			#clock-cells = <0>;
> +		};
> +
> +		tmon_1m_clk: tmon_1m {
> +			compatible = "fixed-clock";
> +			clock-frequency = <1000000>;
> +			#clock-cells = <0>;
> +		};
> +
> +		uartb_clk: uartb {
> +			compatible = "fixed-clock";
> +			clock-frequency = <13000000>;
> +			#clock-cells = <0>;
> +		};
> +
> +		uartb2_clk: uartb2 {
> +			compatible = "fixed-clock";
> +			clock-frequency = <13000000>;
> +			#clock-cells = <0>;
> +		};
> +
> +		uartb3_clk: uartb3 {
> +			compatible = "fixed-clock";
> +			clock-frequency = <13000000>;
> +			#clock-cells = <0>;
> +		};
> +
> +		uartb4_clk: uartb4 {
> +			compatible = "fixed-clock";
> +			clock-frequency = <13000000>;
> +			#clock-cells = <0>;
> +		};
> +
> +		usb_otg_ahb_clk: usb_otg_ahb {
> +			compatible = "fixed-clock";
> +			clock-frequency = <52000000>;
> +			#clock-cells = <0>;
> +		};
> +	};
>   };
>


-- 
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RESEND PATCH v2 1/6] ARM: dts: Declare clocks as fixed on bcm11351
  2013-11-08 10:53   ` Daniel Lezcano
@ 2013-11-08 19:42     ` Tim Kryger
  2013-11-08 21:35       ` Christian Daudt
  0 siblings, 1 reply; 22+ messages in thread
From: Tim Kryger @ 2013-11-08 19:42 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Christian Daudt, Rob Herring, Pawel Moll, Mark Rutland,
	Stephen Warren, Ian Campbell, Thomas Gleixner, Chris Ball,
	Device Tree List, ARM Kernel List, linux-kernel, linux-mmc,
	Patch Tracking

On Fri, Nov 8, 2013 at 2:53 AM, Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:

> I was wondering what is the status of this patchset ?
>
> The patchset touches different areas maintained by different people. Through
> what tree do you expect this patchset to be merged ??

Hi Daniel,

I plan to send an updated version soon to address some concerns raised by Mark.

Once all the maintainers are satisfied and provide their acks, I would
like Christian to take the series into his tree.

I expect Christian will have a few other Broadcom DTS file changes in
his tree so sending the series through him should make it easier to
avoid/manage conflicts.

Christian, would that work for you?

Thanks,
Tim Kryger

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RESEND PATCH v2 1/6] ARM: dts: Declare clocks as fixed on bcm11351
  2013-11-08 19:42     ` Tim Kryger
@ 2013-11-08 21:35       ` Christian Daudt
  0 siblings, 0 replies; 22+ messages in thread
From: Christian Daudt @ 2013-11-08 21:35 UTC (permalink / raw)
  To: Tim Kryger
  Cc: Daniel Lezcano, Rob Herring, Pawel Moll, Mark Rutland,
	Stephen Warren, Ian Campbell, Thomas Gleixner, Chris Ball,
	Device Tree List, ARM Kernel List, linux-kernel, linux-mmc,
	Patch Tracking

I can pick up the dt related changes as long as the other code works
as-is both with and without the change (i.e. no backwards/forwards
compatibility issues). If this must be introduced alongside the other
changes, then the patches will have to be reworked to break them up by
subtree so they can go in atomically.
 csd


On Fri, Nov 8, 2013 at 11:42 AM, Tim Kryger <tim.kryger@linaro.org> wrote:
> On Fri, Nov 8, 2013 at 2:53 AM, Daniel Lezcano
> <daniel.lezcano@linaro.org> wrote:
>
>> I was wondering what is the status of this patchset ?
>>
>> The patchset touches different areas maintained by different people. Through
>> what tree do you expect this patchset to be merged ??
>
> Hi Daniel,
>
> I plan to send an updated version soon to address some concerns raised by Mark.
>
> Once all the maintainers are satisfied and provide their acks, I would
> like Christian to take the series into his tree.
>
> I expect Christian will have a few other Broadcom DTS file changes in
> his tree so sending the series through him should make it easier to
> avoid/manage conflicts.
>
> Christian, would that work for you?
>
> Thanks,
> Tim Kryger

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2013-11-08 21:35 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-16 21:47 [RESEND PATCH v2 0/6] Update Kona drivers to use clocks Tim Kryger
2013-10-16 21:47 ` [RESEND PATCH v2 1/6] ARM: dts: Declare clocks as fixed on bcm11351 Tim Kryger
2013-11-08 10:53   ` Daniel Lezcano
2013-11-08 19:42     ` Tim Kryger
2013-11-08 21:35       ` Christian Daudt
2013-10-16 21:47 ` [RESEND PATCH v2 2/6] ARM: dts: Specify clocks for UARTs " Tim Kryger
2013-10-17  5:41   ` Christian Daudt
2013-10-17  6:14     ` Sebastian Hesselbarth
2013-10-17 13:56   ` Mark Rutland
2013-10-21 18:56     ` Tim Kryger
2013-10-16 21:47 ` [RESEND PATCH v2 3/6] ARM: dts: Specify clocks for SDHCIs " Tim Kryger
2013-10-16 21:47 ` [RESEND PATCH v2 4/6] clocksource: kona: Add basic use of external clock Tim Kryger
2013-10-17 14:05   ` Mark Rutland
2013-10-21 21:07     ` Tim Kryger
2013-10-16 21:47 ` [RESEND PATCH v2 5/6] ARM: dts: Specify clocks for timer on bcm11351 Tim Kryger
2013-10-17 14:06   ` Mark Rutland
2013-10-18 20:25     ` Tim Kryger
2013-10-16 21:47 ` [RESEND PATCH v2 6/6] mmc: sdhci-bcm-kona: Add basic use of clocks Tim Kryger
2013-10-17 14:13   ` Mark Rutland
2013-10-18 13:22     ` Matt Porter
2013-10-21 23:13     ` Tim Kryger
2013-10-17 13:47 ` [RESEND PATCH v2 0/6] Update Kona drivers to use clocks Mark Rutland

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).