Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1234501 > unrolled thread

[PATCH v3 1/4] clk: bcm2835: Move under bcm/ with other Broadcom SoC clk drivers.

Back to article view | Back to linux.kernel

Contents

  [PATCH v3 1/4] clk: bcm2835: Move under bcm/ with other Broadcom SoC clk drivers. Eric Anholt <eric@anholt.net> - 2015-09-28 23:30 +0200
    [PATCH v3 4/4] ARM: bcm2835: Switch to using the new clock driver support. Eric Anholt <eric@anholt.net> - 2015-09-28 23:30 +0200
      Re: [PATCH v3 4/4] ARM: bcm2835: Switch to using the new clock  driver support. Lee Jones <lee@kernel.org> - 2015-10-01 14:30 +0200
    [PATCH v3 2/4] clk: bcm2835: Add binding docs for the new platform clock driver. Eric Anholt <eric@anholt.net> - 2015-09-28 23:30 +0200
      Re: [PATCH v3 2/4] clk: bcm2835: Add binding docs for the new  platform clock driver. Lee Jones <lee@kernel.org> - 2015-10-01 14:30 +0200
      Re: [PATCH v3 2/4] clk: bcm2835: Add binding docs for the new  platform clock driver. Stephen Boyd <sboyd@codeaurora.org> - 2015-10-02 02:20 +0200
    Re: [PATCH v3 3/4] clk: bcm2835: Add support for programming the  audio domain clocks. Stephen Boyd <sboyd@codeaurora.org> - 2015-10-02 02:10 +0200
      Re: [PATCH v3 3/4] clk: bcm2835: Add support for programming the audio domain clocks. Eric Anholt <eric@anholt.net> - 2015-10-02 22:00 +0200
    Re: [PATCH v3 1/4] clk: bcm2835: Move under bcm/ with other Broadcom  SoC clk drivers. Stephen Boyd <sboyd@codeaurora.org> - 2015-10-02 02:20 +0200

#1234501 — [PATCH v3 1/4] clk: bcm2835: Move under bcm/ with other Broadcom SoC clk drivers.

clk-bcm2835.c predates the drivers under bcm/, but all the new BCM
drivers are going in there so let's follow them.

Signed-off-by: Eric Anholt <eric@anholt.net>
Acked-by: Stephen Warren <swarren@wwwdotorg.org>
---
 drivers/clk/Makefile                | 1 -
 drivers/clk/bcm/Makefile            | 1 +
 drivers/clk/{ => bcm}/clk-bcm2835.c | 0
 3 files changed, 1 insertion(+), 1 deletion(-)
 rename drivers/clk/{ => bcm}/clk-bcm2835.c (100%)

diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index d08b3e5..9f71558 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -19,7 +19,6 @@ endif
 obj-$(CONFIG_MACH_ASM9260)		+= clk-asm9260.o
 obj-$(CONFIG_COMMON_CLK_AXI_CLKGEN)	+= clk-axi-clkgen.o
 obj-$(CONFIG_ARCH_AXXIA)		+= clk-axm5516.o
-obj-$(CONFIG_ARCH_BCM2835)		+= clk-bcm2835.o
 obj-$(CONFIG_COMMON_CLK_CDCE706)	+= clk-cdce706.o
 obj-$(CONFIG_ARCH_CLPS711X)		+= clk-clps711x.o
 obj-$(CONFIG_ARCH_EFM32)		+= clk-efm32gg.o
diff --git a/drivers/clk/bcm/Makefile b/drivers/clk/bcm/Makefile
index 8a7a477..ee2349b 100644
--- a/drivers/clk/bcm/Makefile
+++ b/drivers/clk/bcm/Makefile
@@ -3,4 +3,5 @@ obj-$(CONFIG_CLK_BCM_KONA)	+= clk-kona-setup.o
 obj-$(CONFIG_CLK_BCM_KONA)	+= clk-bcm281xx.o
 obj-$(CONFIG_CLK_BCM_KONA)	+= clk-bcm21664.o
 obj-$(CONFIG_COMMON_CLK_IPROC)	+= clk-iproc-armpll.o clk-iproc-pll.o clk-iproc-asiu.o
+obj-$(CONFIG_ARCH_BCM2835)	+= clk-bcm2835.o
 obj-$(CONFIG_ARCH_BCM_CYGNUS)	+= clk-cygnus.o
diff --git a/drivers/clk/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
similarity index 100%
rename from drivers/clk/clk-bcm2835.c
rename to drivers/clk/bcm/clk-bcm2835.c
-- 
2.1.4

--
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/

[toc] | [next] | [standalone]

#1234514 — [PATCH v3 4/4] ARM: bcm2835: Switch to using the new clock driver support.

This will give us the ability to set the pixel and HDMI state machine
clocks for the VC4 KMS driver, change the CPU frequency, and
potentially gate clocks in the future (once we also write a power
domain driver).  It also gives the uart an explicit clock reference,
so that we don't need to change the physical addresses of the old
fixed clk_bcm2835.c clocks for Raspberry Pi 2 port.

Two clocks get their frequencies updated as a result of this.  One is
uart's apb_pclk, which was previously accidentally grabbing the fixed
uart0_pclk due to the apb_pclk not having clk_register_clkdev()
called.  The uart doesn't seem to do anything with apb_pclk other than
make sure it's on, so that appears safe (also, as far as I can see,
the apb clock is actually the same as the VPU clock).  The other is
EMMC, which according to the docs was supposed to be in the 50-100Mhz
range, but it turns out the firmware needed to change to running it at
the 250Mhz core clock speed to avoid a bug in clock domain crossing.

Additionally, anything using BCM2835_CLOCK_VPU will now have a correct
clock rate if the user configures the boot-time core clock speed using
config.txt.

Signed-off-by: Eric Anholt <eric@anholt.net>
Acked-by: Stephen Warren <swarren@wwwdotorg.org>
---
 arch/arm/boot/dts/bcm2835.dtsi | 52 +++++++++++++++++++++++-------------------
 1 file changed, 28 insertions(+), 24 deletions(-)

diff --git a/arch/arm/boot/dts/bcm2835.dtsi b/arch/arm/boot/dts/bcm2835.dtsi
index 301c73f..a6a55b7 100644
--- a/arch/arm/boot/dts/bcm2835.dtsi
+++ b/arch/arm/boot/dts/bcm2835.dtsi
@@ -1,4 +1,5 @@
 #include <dt-bindings/pinctrl/bcm2835.h>
+#include <dt-bindings/clock/bcm2835.h>
 #include "skeleton.dtsi"
 
 / {
@@ -21,6 +22,10 @@
 			compatible = "brcm,bcm2835-system-timer";
 			reg = <0x7e003000 0x1000>;
 			interrupts = <1 0>, <1 1>, <1 2>, <1 3>;
+			/* This could be a reference to BCM2835_CLOCK_TIMER,
+			 * but we don't have the driver using the common clock
+			 * support yet.
+			 */
 			clock-frequency = <1000000>;
 		};
 
@@ -57,6 +62,17 @@
 			reg = <0x7e100000 0x28>;
 		};
 
+		clocks: cprman@7e101000 {
+			compatible = "brcm,bcm2835-cprman";
+			#clock-cells = <1>;
+			reg = <0x7e101000 0x2000>;
+
+			/* CPRMAN derives everything from the platform's
+			 * oscillator.
+			 */
+			clocks = <&clk_osc>;
+		};
+
 		rng@7e104000 {
 			compatible = "brcm,bcm2835-rng";
 			reg = <0x7e104000 0x10>;
@@ -96,7 +112,9 @@
 			compatible = "brcm,bcm2835-pl011", "arm,pl011", "arm,primecell";
 			reg = <0x7e201000 0x1000>;
 			interrupts = <2 25>;
-			clock-frequency = <3000000>;
+			clocks = <&clocks BCM2835_CLOCK_UART>,
+				 <&clocks BCM2835_CLOCK_VPU>;
+			clock-names = "uartclk", "apb_pclk";
 			arm,primecell-periphid = <0x00241011>;
 		};
 
@@ -115,7 +133,7 @@
 			compatible = "brcm,bcm2835-spi";
 			reg = <0x7e204000 0x1000>;
 			interrupts = <2 22>;
-			clocks = <&clk_spi>;
+			clocks = <&clocks BCM2835_CLOCK_VPU>;
 			#address-cells = <1>;
 			#size-cells = <0>;
 			status = "disabled";
@@ -125,7 +143,7 @@
 			compatible = "brcm,bcm2835-i2c";
 			reg = <0x7e205000 0x1000>;
 			interrupts = <2 21>;
-			clocks = <&clk_i2c>;
+			clocks = <&clocks BCM2835_CLOCK_VPU>;
 			#address-cells = <1>;
 			#size-cells = <0>;
 			status = "disabled";
@@ -135,7 +153,7 @@
 			compatible = "brcm,bcm2835-sdhci";
 			reg = <0x7e300000 0x100>;
 			interrupts = <2 30>;
-			clocks = <&clk_mmc>;
+			clocks = <&clocks BCM2835_CLOCK_EMMC>;
 			status = "disabled";
 		};
 
@@ -143,7 +161,7 @@
 			compatible = "brcm,bcm2835-i2c";
 			reg = <0x7e804000 0x1000>;
 			interrupts = <2 21>;
-			clocks = <&clk_i2c>;
+			clocks = <&clocks BCM2835_CLOCK_VPU>;
 			#address-cells = <1>;
 			#size-cells = <0>;
 			status = "disabled";
@@ -165,28 +183,14 @@
 		#address-cells = <1>;
 		#size-cells = <0>;
 
-		clk_mmc: clock@0 {
-			compatible = "fixed-clock";
-			reg = <0>;
-			#clock-cells = <0>;
-			clock-output-names = "mmc";
-			clock-frequency = <100000000>;
-		};
-
-		clk_i2c: clock@1 {
+		/* The oscillator is the root of the clock tree. */
+		clk_osc: clock@3 {
 			compatible = "fixed-clock";
-			reg = <1>;
+			reg = <3>;
 			#clock-cells = <0>;
-			clock-output-names = "i2c";
-			clock-frequency = <250000000>;
+			clock-output-names = "osc";
+			clock-frequency = <19200000>;
 		};
 
-		clk_spi: clock@2 {
-			compatible = "fixed-clock";
-			reg = <2>;
-			#clock-cells = <0>;
-			clock-output-names = "spi";
-			clock-frequency = <250000000>;
-		};
 	};
 };
-- 
2.1.4

--
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/

[toc] | [prev] | [next] | [standalone]

#1237368 — Re: [PATCH v3 4/4] ARM: bcm2835: Switch to using the new clock driver support.

On Mon, 28 Sep 2015, Eric Anholt wrote:

> This will give us the ability to set the pixel and HDMI state machine
> clocks for the VC4 KMS driver, change the CPU frequency, and
> potentially gate clocks in the future (once we also write a power
> domain driver).  It also gives the uart an explicit clock reference,
> so that we don't need to change the physical addresses of the old
> fixed clk_bcm2835.c clocks for Raspberry Pi 2 port.
> 
> Two clocks get their frequencies updated as a result of this.  One is
> uart's apb_pclk, which was previously accidentally grabbing the fixed
> uart0_pclk due to the apb_pclk not having clk_register_clkdev()
> called.  The uart doesn't seem to do anything with apb_pclk other than
> make sure it's on, so that appears safe (also, as far as I can see,
> the apb clock is actually the same as the VPU clock).  The other is
> EMMC, which according to the docs was supposed to be in the 50-100Mhz
> range, but it turns out the firmware needed to change to running it at
> the 250Mhz core clock speed to avoid a bug in clock domain crossing.
> 
> Additionally, anything using BCM2835_CLOCK_VPU will now have a correct
> clock rate if the user configures the boot-time core clock speed using
> config.txt.
> 
> Signed-off-by: Eric Anholt <eric@anholt.net>
> Acked-by: Stephen Warren <swarren@wwwdotorg.org>
> ---
>  arch/arm/boot/dts/bcm2835.dtsi | 52 +++++++++++++++++++++++-------------------
>  1 file changed, 28 insertions(+), 24 deletions(-)

Couple of small nits.

> diff --git a/arch/arm/boot/dts/bcm2835.dtsi b/arch/arm/boot/dts/bcm2835.dtsi
> index 301c73f..a6a55b7 100644
> --- a/arch/arm/boot/dts/bcm2835.dtsi
> +++ b/arch/arm/boot/dts/bcm2835.dtsi
> @@ -1,4 +1,5 @@
>  #include <dt-bindings/pinctrl/bcm2835.h>
> +#include <dt-bindings/clock/bcm2835.h>
>  #include "skeleton.dtsi"
>  
>  / {
> @@ -21,6 +22,10 @@
>  			compatible = "brcm,bcm2835-system-timer";
>  			reg = <0x7e003000 0x1000>;
>  			interrupts = <1 0>, <1 1>, <1 2>, <1 3>;
> +			/* This could be a reference to BCM2835_CLOCK_TIMER,
> +			 * but we don't have the driver using the common clock
> +			 * support yet.
> +			 */

Nit: Please use correct multi-line comment formatting.

From Documentation/CodingStyle:

/*
 * This is the preferred style for multi-line
 * comments in the Linux kernel source code.
 * Please use it consistently.
 *
 * Description:  A column of asterisks on the left side,
 * with beginning and ending almost-blank lines.
 */

>  			clock-frequency = <1000000>;
>  		};
>  
> @@ -57,6 +62,17 @@
>  			reg = <0x7e100000 0x28>;
>  		};
>  
> +		clocks: cprman@7e101000 {
> +			compatible = "brcm,bcm2835-cprman";
> +			#clock-cells = <1>;
> +			reg = <0x7e101000 0x2000>;
> +
> +			/* CPRMAN derives everything from the platform's
> +			 * oscillator.
> +			 */

As above.

> +			clocks = <&clk_osc>;
> +		};
> +
>  		rng@7e104000 {
>  			compatible = "brcm,bcm2835-rng";
>  			reg = <0x7e104000 0x10>;
> @@ -96,7 +112,9 @@
>  			compatible = "brcm,bcm2835-pl011", "arm,pl011", "arm,primecell";
>  			reg = <0x7e201000 0x1000>;
>  			interrupts = <2 25>;
> -			clock-frequency = <3000000>;
> +			clocks = <&clocks BCM2835_CLOCK_UART>,
> +				 <&clocks BCM2835_CLOCK_VPU>;
> +			clock-names = "uartclk", "apb_pclk";
>  			arm,primecell-periphid = <0x00241011>;
>  		};
>  
> @@ -115,7 +133,7 @@
>  			compatible = "brcm,bcm2835-spi";
>  			reg = <0x7e204000 0x1000>;
>  			interrupts = <2 22>;
> -			clocks = <&clk_spi>;
> +			clocks = <&clocks BCM2835_CLOCK_VPU>;
>  			#address-cells = <1>;
>  			#size-cells = <0>;
>  			status = "disabled";
> @@ -125,7 +143,7 @@
>  			compatible = "brcm,bcm2835-i2c";
>  			reg = <0x7e205000 0x1000>;
>  			interrupts = <2 21>;
> -			clocks = <&clk_i2c>;
> +			clocks = <&clocks BCM2835_CLOCK_VPU>;
>  			#address-cells = <1>;
>  			#size-cells = <0>;
>  			status = "disabled";
> @@ -135,7 +153,7 @@
>  			compatible = "brcm,bcm2835-sdhci";
>  			reg = <0x7e300000 0x100>;
>  			interrupts = <2 30>;
> -			clocks = <&clk_mmc>;
> +			clocks = <&clocks BCM2835_CLOCK_EMMC>;
>  			status = "disabled";
>  		};
>  
> @@ -143,7 +161,7 @@
>  			compatible = "brcm,bcm2835-i2c";
>  			reg = <0x7e804000 0x1000>;
>  			interrupts = <2 21>;
> -			clocks = <&clk_i2c>;
> +			clocks = <&clocks BCM2835_CLOCK_VPU>;
>  			#address-cells = <1>;
>  			#size-cells = <0>;
>  			status = "disabled";
> @@ -165,28 +183,14 @@
>  		#address-cells = <1>;
>  		#size-cells = <0>;
>  
> -		clk_mmc: clock@0 {
> -			compatible = "fixed-clock";
> -			reg = <0>;
> -			#clock-cells = <0>;
> -			clock-output-names = "mmc";
> -			clock-frequency = <100000000>;
> -		};
> -
> -		clk_i2c: clock@1 {
> +		/* The oscillator is the root of the clock tree. */
> +		clk_osc: clock@3 {
>  			compatible = "fixed-clock";
> -			reg = <1>;
> +			reg = <3>;
>  			#clock-cells = <0>;
> -			clock-output-names = "i2c";
> -			clock-frequency = <250000000>;
> +			clock-output-names = "osc";
> +			clock-frequency = <19200000>;
>  		};
>  
> -		clk_spi: clock@2 {
> -			compatible = "fixed-clock";
> -			reg = <2>;
> -			#clock-cells = <0>;
> -			clock-output-names = "spi";
> -			clock-frequency = <250000000>;
> -		};
>  	};
>  };
--
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/

[toc] | [prev] | [next] | [standalone]

#1234516 — [PATCH v3 2/4] clk: bcm2835: Add binding docs for the new platform clock driver.

Previously we've only supported a few fixed clocks based on
assumptions about how the firmware sets up the clocks, but this
binding will let us control the actual (audio power domain) clock
manager.

Signed-off-by: Eric Anholt <eric@anholt.net>
Acked-by: Stephen Warren <swarren@wwwdotorg.org>
---
 .../bindings/clock/brcm,bcm2835-cprman.txt         | 45 +++++++++++++++++++++
 include/dt-bindings/clock/bcm2835.h                | 47 ++++++++++++++++++++++
 2 files changed, 92 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/brcm,bcm2835-cprman.txt
 create mode 100644 include/dt-bindings/clock/bcm2835.h

diff --git a/Documentation/devicetree/bindings/clock/brcm,bcm2835-cprman.txt b/Documentation/devicetree/bindings/clock/brcm,bcm2835-cprman.txt
new file mode 100644
index 0000000..e56a1df
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/brcm,bcm2835-cprman.txt
@@ -0,0 +1,45 @@
+Broadcom BCM2835 CPRMAN clocks
+
+This binding uses the common clock binding:
+    Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+The CPRMAN clock controller generates clocks in the audio power domain
+of the BCM2835.  There is a level of PLLs deriving from an external
+oscillator, a level of PLL dividers that produce channels off of the
+few PLLs, and a level of mostly-generic clock generators sourcing from
+the PLL channels.  Most other hardware components source from the
+clock generators, but a few (like the ARM or HDMI) will source from
+the PLL dividers directly.
+
+Required properties:
+- compatible:	Should be "brcm,bcm2835-cprman"
+- #clock-cells:	Should be <1>. The permitted clock-specifier values can be
+		  found in include/dt-bindings/clock/bcm2835.h
+- reg:		Specifies base physical address and size of the registers
+- clocks:	The external oscillator clock phandle
+
+Example:
+
+	clk_osc: clock@3 {
+		compatible = "fixed-clock";
+		reg = <3>;
+		#clock-cells = <0>;
+		clock-output-names = "osc";
+		clock-frequency = <19200000>;
+	};
+
+	clocks: cprman@7e101000 {
+		compatible = "brcm,bcm2835-cprman";
+		#clock-cells = <1>;
+		reg = <0x7e101000 0x2000>;
+		clocks = <&clk_osc>;
+	};
+
+	i2c0: i2c@7e205000 {
+		compatible = "brcm,bcm2835-i2c";
+		reg = <0x7e205000 0x1000>;
+		interrupts = <2 21>;
+		clocks = <&clocks BCM2835_CLOCK_VPU>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+	};
diff --git a/include/dt-bindings/clock/bcm2835.h b/include/dt-bindings/clock/bcm2835.h
new file mode 100644
index 0000000..d323efa
--- /dev/null
+++ b/include/dt-bindings/clock/bcm2835.h
@@ -0,0 +1,47 @@
+/*
+ * Copyright (C) 2015 Broadcom Corporation
+ *
+ * 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 version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#define BCM2835_PLLA			0
+#define BCM2835_PLLB			1
+#define BCM2835_PLLC			2
+#define BCM2835_PLLD			3
+#define BCM2835_PLLH			4
+
+#define BCM2835_PLLA_CORE		5
+#define BCM2835_PLLA_PER		6
+#define BCM2835_PLLB_ARM		7
+#define BCM2835_PLLC_CORE0		8
+#define BCM2835_PLLC_CORE1		9
+#define BCM2835_PLLC_CORE2		10
+#define BCM2835_PLLC_PER		11
+#define BCM2835_PLLD_CORE		12
+#define BCM2835_PLLD_PER		13
+#define BCM2835_PLLH_RCAL		14
+#define BCM2835_PLLH_AUX		15
+#define BCM2835_PLLH_PIX		16
+
+#define BCM2835_CLOCK_TIMER		17
+#define BCM2835_CLOCK_OTP		18
+#define BCM2835_CLOCK_UART		19
+#define BCM2835_CLOCK_VPU		20
+#define BCM2835_CLOCK_V3D		21
+#define BCM2835_CLOCK_ISP		22
+#define BCM2835_CLOCK_H264		23
+#define BCM2835_CLOCK_VEC		24
+#define BCM2835_CLOCK_HSM		25
+#define BCM2835_CLOCK_SDRAM		26
+#define BCM2835_CLOCK_TSENS		27
+#define BCM2835_CLOCK_EMMC		28
+#define BCM2835_CLOCK_PERI_IMAGE	29
+
+#define BCM2835_CLOCK_COUNT		30
-- 
2.1.4

--
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/

[toc] | [prev] | [next] | [standalone]

#1237376 — Re: [PATCH v3 2/4] clk: bcm2835: Add binding docs for the new platform clock driver.

On Mon, 28 Sep 2015, Eric Anholt wrote:

> Previously we've only supported a few fixed clocks based on
> assumptions about how the firmware sets up the clocks, but this
> binding will let us control the actual (audio power domain) clock
> manager.
> 
> Signed-off-by: Eric Anholt <eric@anholt.net>
> Acked-by: Stephen Warren <swarren@wwwdotorg.org>
> ---
>  .../bindings/clock/brcm,bcm2835-cprman.txt         | 45 +++++++++++++++++++++
>  include/dt-bindings/clock/bcm2835.h                | 47 ++++++++++++++++++++++
>  2 files changed, 92 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/brcm,bcm2835-cprman.txt
>  create mode 100644 include/dt-bindings/clock/bcm2835.h

Acked-by: Lee Jones <lee@kernel.org>

> diff --git a/Documentation/devicetree/bindings/clock/brcm,bcm2835-cprman.txt b/Documentation/devicetree/bindings/clock/brcm,bcm2835-cprman.txt
> new file mode 100644
> index 0000000..e56a1df
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/brcm,bcm2835-cprman.txt
> @@ -0,0 +1,45 @@
> +Broadcom BCM2835 CPRMAN clocks
> +
> +This binding uses the common clock binding:
> +    Documentation/devicetree/bindings/clock/clock-bindings.txt
> +
> +The CPRMAN clock controller generates clocks in the audio power domain
> +of the BCM2835.  There is a level of PLLs deriving from an external
> +oscillator, a level of PLL dividers that produce channels off of the
> +few PLLs, and a level of mostly-generic clock generators sourcing from
> +the PLL channels.  Most other hardware components source from the
> +clock generators, but a few (like the ARM or HDMI) will source from
> +the PLL dividers directly.
> +
> +Required properties:
> +- compatible:	Should be "brcm,bcm2835-cprman"
> +- #clock-cells:	Should be <1>. The permitted clock-specifier values can be
> +		  found in include/dt-bindings/clock/bcm2835.h
> +- reg:		Specifies base physical address and size of the registers
> +- clocks:	The external oscillator clock phandle
> +
> +Example:
> +
> +	clk_osc: clock@3 {
> +		compatible = "fixed-clock";
> +		reg = <3>;
> +		#clock-cells = <0>;
> +		clock-output-names = "osc";
> +		clock-frequency = <19200000>;
> +	};
> +
> +	clocks: cprman@7e101000 {
> +		compatible = "brcm,bcm2835-cprman";
> +		#clock-cells = <1>;
> +		reg = <0x7e101000 0x2000>;
> +		clocks = <&clk_osc>;
> +	};
> +
> +	i2c0: i2c@7e205000 {
> +		compatible = "brcm,bcm2835-i2c";
> +		reg = <0x7e205000 0x1000>;
> +		interrupts = <2 21>;
> +		clocks = <&clocks BCM2835_CLOCK_VPU>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +	};
> diff --git a/include/dt-bindings/clock/bcm2835.h b/include/dt-bindings/clock/bcm2835.h
> new file mode 100644
> index 0000000..d323efa
> --- /dev/null
> +++ b/include/dt-bindings/clock/bcm2835.h
> @@ -0,0 +1,47 @@
> +/*
> + * Copyright (C) 2015 Broadcom Corporation
> + *
> + * 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 version 2.
> + *
> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> + * kind, whether express or implied; without even the implied warranty
> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#define BCM2835_PLLA			0
> +#define BCM2835_PLLB			1
> +#define BCM2835_PLLC			2
> +#define BCM2835_PLLD			3
> +#define BCM2835_PLLH			4
> +
> +#define BCM2835_PLLA_CORE		5
> +#define BCM2835_PLLA_PER		6
> +#define BCM2835_PLLB_ARM		7
> +#define BCM2835_PLLC_CORE0		8
> +#define BCM2835_PLLC_CORE1		9
> +#define BCM2835_PLLC_CORE2		10
> +#define BCM2835_PLLC_PER		11
> +#define BCM2835_PLLD_CORE		12
> +#define BCM2835_PLLD_PER		13
> +#define BCM2835_PLLH_RCAL		14
> +#define BCM2835_PLLH_AUX		15
> +#define BCM2835_PLLH_PIX		16
> +
> +#define BCM2835_CLOCK_TIMER		17
> +#define BCM2835_CLOCK_OTP		18
> +#define BCM2835_CLOCK_UART		19
> +#define BCM2835_CLOCK_VPU		20
> +#define BCM2835_CLOCK_V3D		21
> +#define BCM2835_CLOCK_ISP		22
> +#define BCM2835_CLOCK_H264		23
> +#define BCM2835_CLOCK_VEC		24
> +#define BCM2835_CLOCK_HSM		25
> +#define BCM2835_CLOCK_SDRAM		26
> +#define BCM2835_CLOCK_TSENS		27
> +#define BCM2835_CLOCK_EMMC		28
> +#define BCM2835_CLOCK_PERI_IMAGE	29
> +
> +#define BCM2835_CLOCK_COUNT		30
--
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/

[toc] | [prev] | [next] | [standalone]

#1237843 — Re: [PATCH v3 2/4] clk: bcm2835: Add binding docs for the new platform clock driver.

On 09/28, Eric Anholt wrote:
> Previously we've only supported a few fixed clocks based on
> assumptions about how the firmware sets up the clocks, but this
> binding will let us control the actual (audio power domain) clock
> manager.
> 
> Signed-off-by: Eric Anholt <eric@anholt.net>
> Acked-by: Stephen Warren <swarren@wwwdotorg.org>
> ---

Applied to clk-bcm2385 and merged into clk-next.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
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/

[toc] | [prev] | [next] | [standalone]

#1237839 — Re: [PATCH v3 3/4] clk: bcm2835: Add support for programming the audio domain clocks.

On 09/28, Eric Anholt wrote:
> +
> +static const char *bcm2835_clock_per_parents[] = {
> +static const char *bcm2835_clock_vpu_parents[] = {
> +static const char *bcm2835_clock_osc_parents[] = {

Can these parent arrays be const char * const ?

> +	"gnd",
> +	"xosc",
> +	"testdebug0",
> +	"testdebug1"
> +};
> +
> +/*
> + * Used for a 1Mhz clock for the system clocksource, and also used by
> + * the watchdog timer and the camera pulse generator.
> + */
> +static struct bcm2835_clock_data bcm2835_clock_timer_data = {
> +static struct bcm2835_clock_data bcm2835_clock_otp_data = {
> +static struct bcm2835_clock_data bcm2835_clock_vpu_data = {
> +static struct bcm2835_clock_data bcm2835_clock_v3d_data = {
> +static struct bcm2835_clock_data bcm2835_clock_isp_data = {
> +static struct bcm2835_clock_data bcm2835_clock_h264_data = {
> +static struct bcm2835_clock_data bcm2835_clock_vec_data = {
> +static struct bcm2835_clock_data bcm2835_clock_uart_data = {
> +static struct bcm2835_clock_data bcm2835_clock_hsm_data = {
> +static struct bcm2835_clock_data bcm2835_clock_sdram_data = {
> +static struct bcm2835_clock_data bcm2835_clock_tsens_data = {
> +static struct bcm2835_clock_data bcm2835_clock_emmc_data = {

Can all these data structures be const?

> +static int bcm2835_pll_is_on(struct clk_hw *hw)
> +{
> +	struct bcm2835_pll *pll = container_of(hw, struct bcm2835_pll, hw);
> +	struct bcm2835_cprman *cprman = pll->cprman;
> +	const struct bcm2835_pll_data *data = pll->data;
> +
> +	return (cprman_read(cprman, data->a2w_ctrl_reg) &
> +		A2W_PLL_CTRL_PRST_DISABLE);

Useless parenthesis.

> +}
> +
> +static void bcm2835_pll_choose_ndiv_and_fdiv(unsigned long rate,
> +					     unsigned long parent_rate,
> +					     u32 *ndiv, u32 *fdiv)
> +{
> +	u64 div;
> +
> +	div = ((u64)rate << A2W_PLL_FRAC_BITS);
> +	do_div(div, parent_rate);
> +
> +	*ndiv = div >> A2W_PLL_FRAC_BITS;
> +	*fdiv = div & ((1 << A2W_PLL_FRAC_BITS) - 1);
> +}
[..]
> +static unsigned long bcm2835_pll_get_rate(struct clk_hw *hw,
> +					  unsigned long parent_rate)
> +{
> +	struct bcm2835_pll *pll = container_of(hw, struct bcm2835_pll, hw);
> +	struct bcm2835_cprman *cprman = pll->cprman;
> +	const struct bcm2835_pll_data *data = pll->data;
> +	u32 a2wctrl = cprman_read(cprman, data->a2w_ctrl_reg);
> +	u32 ndiv, pdiv, fdiv;
> +
> +	if (parent_rate == 0)
> +		return 0;
> +
> +	fdiv = cprman_read(cprman, data->frac_reg) & A2W_PLL_FRAC_MASK;
> +	ndiv = (a2wctrl & A2W_PLL_CTRL_NDIV_MASK) >> A2W_PLL_CTRL_NDIV_SHIFT;
> +	pdiv = (a2wctrl & A2W_PLL_CTRL_PDIV_MASK) >> A2W_PLL_CTRL_PDIV_SHIFT;
> +
> +	if (cprman_read(cprman, data->ana_reg_base + 4) &
> +	    data->ana->fb_prediv_mask) {
> +		ndiv *= 2;
> +	}

How about a local variable so that we can put the if on one line
and drop the braces?

> +
> +	return bcm2835_pll_rate_from_divisors(parent_rate, ndiv, fdiv, pdiv);
> +}
> +
[..]
> +static int bcm2835_pll_on(struct clk_hw *hw)
> +{
> +	struct bcm2835_pll *pll = container_of(hw, struct bcm2835_pll, hw);
> +	struct bcm2835_cprman *cprman = pll->cprman;
> +	const struct bcm2835_pll_data *data = pll->data;
> +
> +	/* Take the PLL out of reset. */
> +	cprman_write(cprman, data->cm_ctrl_reg,
> +		     cprman_read(cprman, data->cm_ctrl_reg) & ~CM_PLL_ANARST);
> +
> +	/* Wait for the PLL to lock. */
> +	while (!(cprman_read(cprman, CM_LOCK) & data->lock_mask))
> +		cpu_relax();

Is there any reasonable timeout that we can put here? Hopefully
this isn't an infinite loop.

> +
> +	return 0;
> +}
> +
> +static int bcm2835_pll_set_rate(struct clk_hw *hw,
> +				unsigned long rate, unsigned long parent_rate)
> +{
[..]
> +
> +	/* Unmask the reference clock from the oscillator. */
> +	cprman_write(cprman, A2W_XOSC_CTRL,
> +		     cprman_read(cprman, A2W_XOSC_CTRL) |
> +		     data->reference_enable_mask);
> +
> +	if (do_ana_setup_first) {
> +		cprman_write(cprman, data->ana_reg_base + 12, ana3);
> +		cprman_write(cprman, data->ana_reg_base + 8, ana2);

ana2 never changes, so why do we need to write it again?

> +		cprman_write(cprman, data->ana_reg_base + 4, ana1);
> +		cprman_write(cprman, data->ana_reg_base + 0, ana0);

Maybe this should be a function that takes a u32 array of size 4.

> +	}
> +
> +	/* Set the PLL multiplier from the oscillator. */
> +	cprman_write(cprman, data->frac_reg, fdiv);
> +	cprman_write(cprman, data->a2w_ctrl_reg,
> +		     (cprman_read(cprman, data->a2w_ctrl_reg) &
> +		      ~(A2W_PLL_CTRL_NDIV_MASK |
> +			A2W_PLL_CTRL_PDIV_MASK)) |
> +		     (ndiv << A2W_PLL_CTRL_NDIV_SHIFT) |
> +		     (pdiv << A2W_PLL_CTRL_PDIV_SHIFT));

This is a 6 line write. Can we get some local variables and do
the bit setting in different statements?

> +
> +	if (!do_ana_setup_first) {
> +		cprman_write(cprman, data->ana_reg_base + 12, ana3);
> +		cprman_write(cprman, data->ana_reg_base + 8, ana2);
> +		cprman_write(cprman, data->ana_reg_base + 4, ana1);
> +		cprman_write(cprman, data->ana_reg_base + 0, ana0);

Function would help because we do this twice.

> +	}
> +
> +	bcm2835_pll_get_rate(&pll->hw, parent_rate);
> +
> +	return 0;
> +}
> +
[...]
> +static unsigned long bcm2835_pll_divider_get_rate(struct clk_hw *hw,
> +						  unsigned long parent_rate)
> +{
> +	struct bcm2835_pll_divider *divider =
> +		container_of(hw, struct bcm2835_pll_divider, div.hw);
> +	struct bcm2835_cprman *cprman = divider->cprman;
> +	const struct bcm2835_pll_divider_data *data = divider->data;
> +	u32 div = cprman_read(cprman, data->a2w_reg);
> +
> +	div &= ((1 << A2W_PLL_DIV_BITS) - 1);

One too many parenthesis here.

> +	if (div == 0)
> +		div = 256;
> +
> +	return parent_rate / div;
> +}
> +
[..]
> +
> +static int bcm2835_clock_is_on(struct clk_hw *hw)
> +{
> +	struct bcm2835_clock *clock =
> +		container_of(hw, struct bcm2835_clock, hw);
> +	struct bcm2835_cprman *cprman = clock->cprman;
> +	const struct bcm2835_clock_data *data = clock->data;
> +
> +	/*
> +	 * The VPU clock is always on, regardless of what we might set
> +	 * the enable bit to.
> +	 */
> +	if (data->is_nonstop)

Maybe the variable should be called is_vpu_clock then? Or the
comment is going to go out of date soon.

> +		return true;
> +
> +	return (cprman_read(cprman, data->ctl_reg) & CM_ENABLE) != 0;
> +}
> +
> +static u32 bcm2835_clock_choose_div(struct clk_hw *hw,
> +				    unsigned long rate,
> +				    unsigned long parent_rate)
> +{
> +	struct bcm2835_clock *clock =
> +		container_of(hw, struct bcm2835_clock, hw);
> +	const struct bcm2835_clock_data *data = clock->data;
> +	u32 unused_frac_mask = (1 << (CM_DIV_FRAC_BITS - data->frac_bits)) - 1;

We have GENMASK for this sort of stuff.

> +	u64 temp = (u64)parent_rate << CM_DIV_FRAC_BITS;
> +	u32 div;
> +
> +	do_div(temp, rate);
> +	div = temp;
> +
> +	/* Round and mask off the unused bits */
> +	if (unused_frac_mask != 0) {
> +		div += unused_frac_mask >> 1;
> +		div &= ~unused_frac_mask;
> +	}
> +
> +	/* Clamp to the limits. */
> +	div = max(div, unused_frac_mask + 1);
> +	div = min(div, (((1 << (data->int_bits + CM_DIV_FRAC_BITS)) - 1)) &
> +		  ~unused_frac_mask);
> +
> +	return div;
> +}
> +
> +static long bcm2835_clock_rate_from_divisor(struct bcm2835_clock *clock,
> +					    unsigned long parent_rate,
> +					    u32 div)
> +{
> +	const struct bcm2835_clock_data *data = clock->data;
> +	u64 temp;
> +
> +	/*
> +	 * The divisor is a 12.12 fixed point field, but only some of
> +	 * the bits are populated in any given clock.
> +	 */
> +	div >>= (CM_DIV_FRAC_BITS - data->frac_bits);

Useless parenthesis here.

> +	div &= (1 << (data->int_bits + data->frac_bits)) - 1;
> +
> +	if (div == 0)
> +		return 0;
> +
> +	temp = (u64)parent_rate << data->frac_bits;
> +
> +	do_div(temp, div);
> +
> +	return temp;
> +}
> +
> +static long bcm2835_clock_round_rate(struct clk_hw *hw,
> +				     unsigned long rate,
> +				     unsigned long *parent_rate)
> +{
> +	struct bcm2835_clock *clock =
> +		container_of(hw, struct bcm2835_clock, hw);

Would be nice to have a macro to get this onto one line

	struct bcm2835_clock *clock = to_bcm2385_clock(hw);

> +	u32 div = bcm2835_clock_choose_div(hw, rate, *parent_rate);
> +
> +	return bcm2835_clock_rate_from_divisor(clock, *parent_rate, div);
> +}
> +
> +static unsigned long bcm2835_clock_get_rate(struct clk_hw *hw,
> +					    unsigned long parent_rate)
> +{
> +	struct bcm2835_clock *clock =
> +		container_of(hw, struct bcm2835_clock, hw);
> +	struct bcm2835_cprman *cprman = clock->cprman;
> +	const struct bcm2835_clock_data *data = clock->data;
> +	u32 div = cprman_read(cprman, data->div_reg);
> +
> +	return bcm2835_clock_rate_from_divisor(clock, parent_rate, div);
> +}
> +
> +static void bcm2835_clock_wait_busy(struct bcm2835_clock *clock)
> +{
> +	struct bcm2835_cprman *cprman = clock->cprman;
> +	const struct bcm2835_clock_data *data = clock->data;
> +
> +	while (cprman_read(cprman, data->ctl_reg) & CM_BUSY)
> +		cpu_relax();
> +}
> +
> +static void bcm2835_clock_off(struct clk_hw *hw)
> +{
> +	struct bcm2835_clock *clock =
> +		container_of(hw, struct bcm2835_clock, hw);
> +	struct bcm2835_cprman *cprman = clock->cprman;
> +	const struct bcm2835_clock_data *data = clock->data;
> +
> +	if (data->is_nonstop)

Or we should have different clk_ops for clocks that are "nonstop" so
that we don't do any sorts of checks here.

> +		return;
> +
> +	spin_lock(&cprman->regs_lock);
> +	cprman_write(cprman, data->ctl_reg,
> +		     cprman_read(cprman, data->ctl_reg) & ~CM_ENABLE);
> +	spin_unlock(&cprman->regs_lock);
> +
> +	/* BUSY will remain high until the divider completes its cycle. */
> +	bcm2835_clock_wait_busy(clock);
> +}
> +
[..]
> +static struct clk *
> +bcm2835_register_pll_divider(struct bcm2835_cprman *cprman,
> +			     const struct bcm2835_pll_divider_data *data)
> +{
[..]
> +	clk = clk_register(cprman->dev, &divider->div.hw);

What if clk_register() fails?

> +
> +	/*
> +	 * PLLH's channels have a fixed divide by 10 afterwards, which
> +	 * is what our consumers are actually using.
> +	 */
> +	if (data->fixed_divider != 1) {
> +		return clk_register_fixed_factor(cprman->dev, data->name,
> +						 divider_name,
> +						 CLK_SET_RATE_PARENT,
> +						 1,
> +						 data->fixed_divider);
> +	} else {
> +		return clk;
> +	}

Just return clk instead of the else return clk compound
statement.

> +}
> +
> +static struct clk *bcm2835_register_clock(struct bcm2835_cprman *cprman,
> +					  const struct bcm2835_clock_data *data)
> +{
> +	struct bcm2835_clock *clock;
> +	struct clk_init_data init;
> +	const char *parent;
> +
> +	/*
> +	 * Most of the clock generators have a mux field, so we
> +	 * instantiate a generic mux as our parent to handle it.
> +	 */
> +	if (data->num_mux_parents) {
> +		int i;
> +
> +		parent = kasprintf(GFP_KERNEL, "mux_%s", data->name);
> +		if (!parent)
> +			return NULL;
> +
> +		/*
> +		 * Replace our "xosc" references with the actual
> +		 * oscillator's name.
> +		 */
> +		for (i = 0; i < data->num_mux_parents; i++) {
> +			if (strcmp(data->parents[i], "xosc") == 0)
> +				data->parents[i] = cprman->osc_name;
> +		}

Braces aren't needed here.


-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
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/

[toc] | [prev] | [next] | [standalone]

#1238566 — Re: [PATCH v3 3/4] clk: bcm2835: Add support for programming the audio domain clocks.

[Multipart message — attachments visible in raw view] — view raw

Stephen Boyd <sboyd@codeaurora.org> writes:

> On 09/28, Eric Anholt wrote:
>> +
>> +static const char *bcm2835_clock_per_parents[] = {
>> +static const char *bcm2835_clock_vpu_parents[] = {
>> +static const char *bcm2835_clock_osc_parents[] = {
>
> Can these parent arrays be const char * const ?

They couldn't because I was editing them in place to do the
s/"xosc"/of_clk_get_parent_name()/ edit.  I've moved the edit to being
in a stack allocation at probe time.

I think I've done all of your suggested changes in the new version (and
explained the extra ana2 read/write).

[toc] | [prev] | [next] | [standalone]

#1237844 — Re: [PATCH v3 1/4] clk: bcm2835: Move under bcm/ with other Broadcom SoC clk drivers.

On 09/28, Eric Anholt wrote:
> clk-bcm2835.c predates the drivers under bcm/, but all the new BCM
> drivers are going in there so let's follow them.
> 
> Signed-off-by: Eric Anholt <eric@anholt.net>
> Acked-by: Stephen Warren <swarren@wwwdotorg.org>
> ---

Applied to clk-bcm2385 and merged into clk-next.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
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/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web