All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/2] clk/zynq/clkc: Add 'fclk-enable' feature
@ 2013-10-10 17:10 ` Soren Brinkmann
  0 siblings, 0 replies; 24+ messages in thread
From: Soren Brinkmann @ 2013-10-10 17:10 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
	Ian Campbell, Rob Landley, Russell King, Mike Turquette,
	Michal Simek
  Cc: linux-kernel, linux-arm-kernel, linux-doc, devicetree, Soren Brinkmann

In some use cases Zynq's FPGA clocks are used as static clock
generators for IP in the FPGA part of the SOC for which no Linux driver
exists and would control those clocks. To avoid automatic
gating of these clocks in such cases a new property - fclk-enable - is
added to the clock controller's DT description to accomodate such use
cases. It's value is a bitmask, where a set bit results in enabling
the corresponding FCLK through the clkc.

FPGA clocks are handled following the rules below:

If an FCLK is not enabled by bootloaders, that FCLK will be disabled in
Linux. Drivers can enable and control it through the CCF as usual.

If an FCLK is enabled by bootloaders AND the corresponding bit in the
'fclk-enable' DT property is set, that FCLK will be enabled by the clkc,
resulting in an off by one reference count for that clock. Ensuring it
will always be running.

Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
---
v2:
 - change default value for fclk-enable to '0'
---
 Documentation/devicetree/bindings/clock/zynq-7000.txt |  4 ++++
 drivers/clk/zynq/clkc.c                               | 18 +++++++++++++++---
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/zynq-7000.txt b/Documentation/devicetree/bindings/clock/zynq-7000.txt
index d99af878f5d7..11fdd146ec83 100644
--- a/Documentation/devicetree/bindings/clock/zynq-7000.txt
+++ b/Documentation/devicetree/bindings/clock/zynq-7000.txt
@@ -22,6 +22,10 @@ Required properties:
 Optional properties:
  - clocks : as described in the clock bindings
  - clock-names : as described in the clock bindings
+ - fclk-enable : Bit mask to enable FCLKs in cases no proper CCF compatible
+		 driver is available. Bit [0..3] correspond to FCLK0..FCLK3. The
+		 corresponding FCLK will only be enabled if it is actually
+		 running at boot time. (default = 0xf)
 
 Clock inputs:
 The following strings are optional parameters to the 'clock-names' property in
diff --git a/drivers/clk/zynq/clkc.c b/drivers/clk/zynq/clkc.c
index 10772aa72e4e..af3bd0aec538 100644
--- a/drivers/clk/zynq/clkc.c
+++ b/drivers/clk/zynq/clkc.c
@@ -102,9 +102,10 @@ static const char *swdt_ext_clk_input_names[] __initdata = {"swdt_ext_clk"};
 
 static void __init zynq_clk_register_fclk(enum zynq_clk fclk,
 		const char *clk_name, void __iomem *fclk_ctrl_reg,
-		const char **parents)
+		const char **parents, int enable)
 {
 	struct clk *clk;
+	u32 enable_reg;
 	char *mux_name;
 	char *div0_name;
 	char *div1_name;
@@ -147,6 +148,12 @@ static void __init zynq_clk_register_fclk(enum zynq_clk fclk,
 	clks[fclk] = clk_register_gate(NULL, clk_name,
 			div1_name, CLK_SET_RATE_PARENT, fclk_gate_reg,
 			0, CLK_GATE_SET_TO_DISABLE, fclk_gate_lock);
+	enable_reg = readl(fclk_gate_reg) & 1;
+	if (enable & !enable_reg) {
+		if (clk_prepare_enable(clks[fclk]))
+			pr_warn("%s: FCLK%u enable failed\n", __func__,
+					fclk - fclk0);
+	}
 	kfree(mux_name);
 	kfree(div0_name);
 	kfree(div1_name);
@@ -213,6 +220,7 @@ static void __init zynq_clk_setup(struct device_node *np)
 	int ret;
 	struct clk *clk;
 	char *clk_name;
+	unsigned int fclk_enable = 0;
 	const char *clk_output_name[clk_max];
 	const char *cpu_parents[4];
 	const char *periph_parents[4];
@@ -238,6 +246,8 @@ static void __init zynq_clk_setup(struct device_node *np)
 	periph_parents[2] = clk_output_name[armpll];
 	periph_parents[3] = clk_output_name[ddrpll];
 
+	of_property_read_u32(np, "fclk-enable", &fclk_enable);
+
 	/* ps_clk */
 	ret = of_property_read_u32(np, "ps-clk-frequency", &tmp);
 	if (ret) {
@@ -340,10 +350,12 @@ static void __init zynq_clk_setup(struct device_node *np)
 	clk_prepare_enable(clks[dci]);
 
 	/* Peripheral clocks */
-	for (i = fclk0; i <= fclk3; i++)
+	for (i = fclk0; i <= fclk3; i++) {
+		int enable = !!(fclk_enable & BIT(i - fclk0));
 		zynq_clk_register_fclk(i, clk_output_name[i],
 				SLCR_FPGA0_CLK_CTRL + 0x10 * (i - fclk0),
-				periph_parents);
+				periph_parents, enable);
+	}
 
 	zynq_clk_register_periph_clk(lqspi, 0, clk_output_name[lqspi], NULL,
 			SLCR_LQSPI_CLK_CTRL, periph_parents, 0);
-- 
1.8.4


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

* [PATCH v2 1/2] clk/zynq/clkc: Add 'fclk-enable' feature
@ 2013-10-10 17:10 ` Soren Brinkmann
  0 siblings, 0 replies; 24+ messages in thread
From: Soren Brinkmann @ 2013-10-10 17:10 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
	Ian Campbell, Rob Landley, Russell King, Mike Turquette,
	Michal Simek
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-doc-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Soren Brinkmann

In some use cases Zynq's FPGA clocks are used as static clock
generators for IP in the FPGA part of the SOC for which no Linux driver
exists and would control those clocks. To avoid automatic
gating of these clocks in such cases a new property - fclk-enable - is
added to the clock controller's DT description to accomodate such use
cases. It's value is a bitmask, where a set bit results in enabling
the corresponding FCLK through the clkc.

FPGA clocks are handled following the rules below:

If an FCLK is not enabled by bootloaders, that FCLK will be disabled in
Linux. Drivers can enable and control it through the CCF as usual.

If an FCLK is enabled by bootloaders AND the corresponding bit in the
'fclk-enable' DT property is set, that FCLK will be enabled by the clkc,
resulting in an off by one reference count for that clock. Ensuring it
will always be running.

Signed-off-by: Soren Brinkmann <soren.brinkmann-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
---
v2:
 - change default value for fclk-enable to '0'
---
 Documentation/devicetree/bindings/clock/zynq-7000.txt |  4 ++++
 drivers/clk/zynq/clkc.c                               | 18 +++++++++++++++---
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/zynq-7000.txt b/Documentation/devicetree/bindings/clock/zynq-7000.txt
index d99af878f5d7..11fdd146ec83 100644
--- a/Documentation/devicetree/bindings/clock/zynq-7000.txt
+++ b/Documentation/devicetree/bindings/clock/zynq-7000.txt
@@ -22,6 +22,10 @@ Required properties:
 Optional properties:
  - clocks : as described in the clock bindings
  - clock-names : as described in the clock bindings
+ - fclk-enable : Bit mask to enable FCLKs in cases no proper CCF compatible
+		 driver is available. Bit [0..3] correspond to FCLK0..FCLK3. The
+		 corresponding FCLK will only be enabled if it is actually
+		 running at boot time. (default = 0xf)
 
 Clock inputs:
 The following strings are optional parameters to the 'clock-names' property in
diff --git a/drivers/clk/zynq/clkc.c b/drivers/clk/zynq/clkc.c
index 10772aa72e4e..af3bd0aec538 100644
--- a/drivers/clk/zynq/clkc.c
+++ b/drivers/clk/zynq/clkc.c
@@ -102,9 +102,10 @@ static const char *swdt_ext_clk_input_names[] __initdata = {"swdt_ext_clk"};
 
 static void __init zynq_clk_register_fclk(enum zynq_clk fclk,
 		const char *clk_name, void __iomem *fclk_ctrl_reg,
-		const char **parents)
+		const char **parents, int enable)
 {
 	struct clk *clk;
+	u32 enable_reg;
 	char *mux_name;
 	char *div0_name;
 	char *div1_name;
@@ -147,6 +148,12 @@ static void __init zynq_clk_register_fclk(enum zynq_clk fclk,
 	clks[fclk] = clk_register_gate(NULL, clk_name,
 			div1_name, CLK_SET_RATE_PARENT, fclk_gate_reg,
 			0, CLK_GATE_SET_TO_DISABLE, fclk_gate_lock);
+	enable_reg = readl(fclk_gate_reg) & 1;
+	if (enable & !enable_reg) {
+		if (clk_prepare_enable(clks[fclk]))
+			pr_warn("%s: FCLK%u enable failed\n", __func__,
+					fclk - fclk0);
+	}
 	kfree(mux_name);
 	kfree(div0_name);
 	kfree(div1_name);
@@ -213,6 +220,7 @@ static void __init zynq_clk_setup(struct device_node *np)
 	int ret;
 	struct clk *clk;
 	char *clk_name;
+	unsigned int fclk_enable = 0;
 	const char *clk_output_name[clk_max];
 	const char *cpu_parents[4];
 	const char *periph_parents[4];
@@ -238,6 +246,8 @@ static void __init zynq_clk_setup(struct device_node *np)
 	periph_parents[2] = clk_output_name[armpll];
 	periph_parents[3] = clk_output_name[ddrpll];
 
+	of_property_read_u32(np, "fclk-enable", &fclk_enable);
+
 	/* ps_clk */
 	ret = of_property_read_u32(np, "ps-clk-frequency", &tmp);
 	if (ret) {
@@ -340,10 +350,12 @@ static void __init zynq_clk_setup(struct device_node *np)
 	clk_prepare_enable(clks[dci]);
 
 	/* Peripheral clocks */
-	for (i = fclk0; i <= fclk3; i++)
+	for (i = fclk0; i <= fclk3; i++) {
+		int enable = !!(fclk_enable & BIT(i - fclk0));
 		zynq_clk_register_fclk(i, clk_output_name[i],
 				SLCR_FPGA0_CLK_CTRL + 0x10 * (i - fclk0),
-				periph_parents);
+				periph_parents, enable);
+	}
 
 	zynq_clk_register_periph_clk(lqspi, 0, clk_output_name[lqspi], NULL,
 			SLCR_LQSPI_CLK_CTRL, periph_parents, 0);
-- 
1.8.4

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

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

* [PATCH v2 1/2] clk/zynq/clkc: Add 'fclk-enable' feature
@ 2013-10-10 17:10 ` Soren Brinkmann
  0 siblings, 0 replies; 24+ messages in thread
From: Soren Brinkmann @ 2013-10-10 17:10 UTC (permalink / raw)
  To: linux-arm-kernel

In some use cases Zynq's FPGA clocks are used as static clock
generators for IP in the FPGA part of the SOC for which no Linux driver
exists and would control those clocks. To avoid automatic
gating of these clocks in such cases a new property - fclk-enable - is
added to the clock controller's DT description to accomodate such use
cases. It's value is a bitmask, where a set bit results in enabling
the corresponding FCLK through the clkc.

FPGA clocks are handled following the rules below:

If an FCLK is not enabled by bootloaders, that FCLK will be disabled in
Linux. Drivers can enable and control it through the CCF as usual.

If an FCLK is enabled by bootloaders AND the corresponding bit in the
'fclk-enable' DT property is set, that FCLK will be enabled by the clkc,
resulting in an off by one reference count for that clock. Ensuring it
will always be running.

Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
---
v2:
 - change default value for fclk-enable to '0'
---
 Documentation/devicetree/bindings/clock/zynq-7000.txt |  4 ++++
 drivers/clk/zynq/clkc.c                               | 18 +++++++++++++++---
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/zynq-7000.txt b/Documentation/devicetree/bindings/clock/zynq-7000.txt
index d99af878f5d7..11fdd146ec83 100644
--- a/Documentation/devicetree/bindings/clock/zynq-7000.txt
+++ b/Documentation/devicetree/bindings/clock/zynq-7000.txt
@@ -22,6 +22,10 @@ Required properties:
 Optional properties:
  - clocks : as described in the clock bindings
  - clock-names : as described in the clock bindings
+ - fclk-enable : Bit mask to enable FCLKs in cases no proper CCF compatible
+		 driver is available. Bit [0..3] correspond to FCLK0..FCLK3. The
+		 corresponding FCLK will only be enabled if it is actually
+		 running at boot time. (default = 0xf)
 
 Clock inputs:
 The following strings are optional parameters to the 'clock-names' property in
diff --git a/drivers/clk/zynq/clkc.c b/drivers/clk/zynq/clkc.c
index 10772aa72e4e..af3bd0aec538 100644
--- a/drivers/clk/zynq/clkc.c
+++ b/drivers/clk/zynq/clkc.c
@@ -102,9 +102,10 @@ static const char *swdt_ext_clk_input_names[] __initdata = {"swdt_ext_clk"};
 
 static void __init zynq_clk_register_fclk(enum zynq_clk fclk,
 		const char *clk_name, void __iomem *fclk_ctrl_reg,
-		const char **parents)
+		const char **parents, int enable)
 {
 	struct clk *clk;
+	u32 enable_reg;
 	char *mux_name;
 	char *div0_name;
 	char *div1_name;
@@ -147,6 +148,12 @@ static void __init zynq_clk_register_fclk(enum zynq_clk fclk,
 	clks[fclk] = clk_register_gate(NULL, clk_name,
 			div1_name, CLK_SET_RATE_PARENT, fclk_gate_reg,
 			0, CLK_GATE_SET_TO_DISABLE, fclk_gate_lock);
+	enable_reg = readl(fclk_gate_reg) & 1;
+	if (enable & !enable_reg) {
+		if (clk_prepare_enable(clks[fclk]))
+			pr_warn("%s: FCLK%u enable failed\n", __func__,
+					fclk - fclk0);
+	}
 	kfree(mux_name);
 	kfree(div0_name);
 	kfree(div1_name);
@@ -213,6 +220,7 @@ static void __init zynq_clk_setup(struct device_node *np)
 	int ret;
 	struct clk *clk;
 	char *clk_name;
+	unsigned int fclk_enable = 0;
 	const char *clk_output_name[clk_max];
 	const char *cpu_parents[4];
 	const char *periph_parents[4];
@@ -238,6 +246,8 @@ static void __init zynq_clk_setup(struct device_node *np)
 	periph_parents[2] = clk_output_name[armpll];
 	periph_parents[3] = clk_output_name[ddrpll];
 
+	of_property_read_u32(np, "fclk-enable", &fclk_enable);
+
 	/* ps_clk */
 	ret = of_property_read_u32(np, "ps-clk-frequency", &tmp);
 	if (ret) {
@@ -340,10 +350,12 @@ static void __init zynq_clk_setup(struct device_node *np)
 	clk_prepare_enable(clks[dci]);
 
 	/* Peripheral clocks */
-	for (i = fclk0; i <= fclk3; i++)
+	for (i = fclk0; i <= fclk3; i++) {
+		int enable = !!(fclk_enable & BIT(i - fclk0));
 		zynq_clk_register_fclk(i, clk_output_name[i],
 				SLCR_FPGA0_CLK_CTRL + 0x10 * (i - fclk0),
-				periph_parents);
+				periph_parents, enable);
+	}
 
 	zynq_clk_register_periph_clk(lqspi, 0, clk_output_name[lqspi], NULL,
 			SLCR_LQSPI_CLK_CTRL, periph_parents, 0);
-- 
1.8.4

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

* [PATCH v2 2/2] arm: dt: zynq: Add fclk-enable property to clkc node
  2013-10-10 17:10 ` Soren Brinkmann
@ 2013-10-10 17:10   ` Soren Brinkmann
  -1 siblings, 0 replies; 24+ messages in thread
From: Soren Brinkmann @ 2013-10-10 17:10 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
	Ian Campbell, Rob Landley, Russell King, Mike Turquette,
	Michal Simek
  Cc: linux-kernel, linux-arm-kernel, linux-doc, devicetree, Soren Brinkmann

Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
---
This is kind of optional since it does not have any effect due to the changed
default in 1/2.

v2:
 - no change


 arch/arm/boot/dts/zynq-7000.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
index e32b92b949d2..b48d0403537b 100644
--- a/arch/arm/boot/dts/zynq-7000.dtsi
+++ b/arch/arm/boot/dts/zynq-7000.dtsi
@@ -77,6 +77,7 @@
 					#clock-cells = <1>;
 					compatible = "xlnx,ps7-clkc";
 					ps-clk-frequency = <33333333>;
+					fclk-enable = <0>;
 					clock-output-names = "armpll", "ddrpll", "iopll", "cpu_6or4x",
 							"cpu_3or2x", "cpu_2x", "cpu_1x", "ddr2x", "ddr3x",
 							"dci", "lqspi", "smc", "pcap", "gem0", "gem1",
-- 
1.8.4


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

* [PATCH v2 2/2] arm: dt: zynq: Add fclk-enable property to clkc node
@ 2013-10-10 17:10   ` Soren Brinkmann
  0 siblings, 0 replies; 24+ messages in thread
From: Soren Brinkmann @ 2013-10-10 17:10 UTC (permalink / raw)
  To: linux-arm-kernel

Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
---
This is kind of optional since it does not have any effect due to the changed
default in 1/2.

v2:
 - no change


 arch/arm/boot/dts/zynq-7000.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
index e32b92b949d2..b48d0403537b 100644
--- a/arch/arm/boot/dts/zynq-7000.dtsi
+++ b/arch/arm/boot/dts/zynq-7000.dtsi
@@ -77,6 +77,7 @@
 					#clock-cells = <1>;
 					compatible = "xlnx,ps7-clkc";
 					ps-clk-frequency = <33333333>;
+					fclk-enable = <0>;
 					clock-output-names = "armpll", "ddrpll", "iopll", "cpu_6or4x",
 							"cpu_3or2x", "cpu_2x", "cpu_1x", "ddr2x", "ddr3x",
 							"dci", "lqspi", "smc", "pcap", "gem0", "gem1",
-- 
1.8.4

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

* Re: [PATCH v2 1/2] clk/zynq/clkc: Add 'fclk-enable' feature
  2013-10-10 17:10 ` Soren Brinkmann
@ 2013-10-14  9:04   ` Michal Simek
  -1 siblings, 0 replies; 24+ messages in thread
From: Michal Simek @ 2013-10-14  9:04 UTC (permalink / raw)
  To: Soren Brinkmann
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
	Ian Campbell, Rob Landley, Russell King, Mike Turquette,
	Michal Simek, linux-kernel, linux-arm-kernel, linux-doc,
	devicetree

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

On 10/10/2013 07:10 PM, Soren Brinkmann wrote:
> In some use cases Zynq's FPGA clocks are used as static clock
> generators for IP in the FPGA part of the SOC for which no Linux driver
> exists and would control those clocks. To avoid automatic
> gating of these clocks in such cases a new property - fclk-enable - is
> added to the clock controller's DT description to accomodate such use
> cases. It's value is a bitmask, where a set bit results in enabling
> the corresponding FCLK through the clkc.
> 
> FPGA clocks are handled following the rules below:
> 
> If an FCLK is not enabled by bootloaders, that FCLK will be disabled in
> Linux. Drivers can enable and control it through the CCF as usual.
> 
> If an FCLK is enabled by bootloaders AND the corresponding bit in the
> 'fclk-enable' DT property is set, that FCLK will be enabled by the clkc,
> resulting in an off by one reference count for that clock. Ensuring it
> will always be running.
> 
> Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
> ---
> v2:
>  - change default value for fclk-enable to '0'
> ---
>  Documentation/devicetree/bindings/clock/zynq-7000.txt |  4 ++++
>  drivers/clk/zynq/clkc.c                               | 18 +++++++++++++++---
>  2 files changed, 19 insertions(+), 3 deletions(-)

For both patches:
Acked-by: Michal Simek <michal.simek@xilinx.com>

Mike: Can you please add both these patches to your tree?
There shouldn't be any conflict with DT patch itself.
But if you don't want to add 2/2 through your tree, I am also fine
with taking this through my zynq tree.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

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

* [PATCH v2 1/2] clk/zynq/clkc: Add 'fclk-enable' feature
@ 2013-10-14  9:04   ` Michal Simek
  0 siblings, 0 replies; 24+ messages in thread
From: Michal Simek @ 2013-10-14  9:04 UTC (permalink / raw)
  To: linux-arm-kernel

On 10/10/2013 07:10 PM, Soren Brinkmann wrote:
> In some use cases Zynq's FPGA clocks are used as static clock
> generators for IP in the FPGA part of the SOC for which no Linux driver
> exists and would control those clocks. To avoid automatic
> gating of these clocks in such cases a new property - fclk-enable - is
> added to the clock controller's DT description to accomodate such use
> cases. It's value is a bitmask, where a set bit results in enabling
> the corresponding FCLK through the clkc.
> 
> FPGA clocks are handled following the rules below:
> 
> If an FCLK is not enabled by bootloaders, that FCLK will be disabled in
> Linux. Drivers can enable and control it through the CCF as usual.
> 
> If an FCLK is enabled by bootloaders AND the corresponding bit in the
> 'fclk-enable' DT property is set, that FCLK will be enabled by the clkc,
> resulting in an off by one reference count for that clock. Ensuring it
> will always be running.
> 
> Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
> ---
> v2:
>  - change default value for fclk-enable to '0'
> ---
>  Documentation/devicetree/bindings/clock/zynq-7000.txt |  4 ++++
>  drivers/clk/zynq/clkc.c                               | 18 +++++++++++++++---
>  2 files changed, 19 insertions(+), 3 deletions(-)

For both patches:
Acked-by: Michal Simek <michal.simek@xilinx.com>

Mike: Can you please add both these patches to your tree?
There shouldn't be any conflict with DT patch itself.
But if you don't want to add 2/2 through your tree, I am also fine
with taking this through my zynq tree.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 263 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131014/3feca7a2/attachment.sig>

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

* Re: [PATCH v2 1/2] clk/zynq/clkc: Add 'fclk-enable' feature
  2013-10-10 17:10 ` Soren Brinkmann
  (?)
@ 2013-10-28 20:59   ` Sören Brinkmann
  -1 siblings, 0 replies; 24+ messages in thread
From: Sören Brinkmann @ 2013-10-28 20:59 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
	Ian Campbell, Rob Landley, Russell King, Mike Turquette,
	Michal Simek
  Cc: linux-kernel, linux-arm-kernel, linux-doc, devicetree

ping?

On Thu, Oct 10, 2013 at 10:10:17AM -0700, Soren Brinkmann wrote:
> In some use cases Zynq's FPGA clocks are used as static clock
> generators for IP in the FPGA part of the SOC for which no Linux driver
> exists and would control those clocks. To avoid automatic
> gating of these clocks in such cases a new property - fclk-enable - is
> added to the clock controller's DT description to accomodate such use
> cases. It's value is a bitmask, where a set bit results in enabling
> the corresponding FCLK through the clkc.
> 
> FPGA clocks are handled following the rules below:
> 
> If an FCLK is not enabled by bootloaders, that FCLK will be disabled in
> Linux. Drivers can enable and control it through the CCF as usual.
> 
> If an FCLK is enabled by bootloaders AND the corresponding bit in the
> 'fclk-enable' DT property is set, that FCLK will be enabled by the clkc,
> resulting in an off by one reference count for that clock. Ensuring it
> will always be running.
> 
> Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
> ---
> v2:
>  - change default value for fclk-enable to '0'
> ---
>  Documentation/devicetree/bindings/clock/zynq-7000.txt |  4 ++++
>  drivers/clk/zynq/clkc.c                               | 18 +++++++++++++++---
>  2 files changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/clock/zynq-7000.txt b/Documentation/devicetree/bindings/clock/zynq-7000.txt
> index d99af878f5d7..11fdd146ec83 100644
> --- a/Documentation/devicetree/bindings/clock/zynq-7000.txt
> +++ b/Documentation/devicetree/bindings/clock/zynq-7000.txt
> @@ -22,6 +22,10 @@ Required properties:
>  Optional properties:
>   - clocks : as described in the clock bindings
>   - clock-names : as described in the clock bindings
> + - fclk-enable : Bit mask to enable FCLKs in cases no proper CCF compatible
> +		 driver is available. Bit [0..3] correspond to FCLK0..FCLK3. The
> +		 corresponding FCLK will only be enabled if it is actually
> +		 running at boot time. (default = 0xf)
>  
>  Clock inputs:
>  The following strings are optional parameters to the 'clock-names' property in
> diff --git a/drivers/clk/zynq/clkc.c b/drivers/clk/zynq/clkc.c
> index 10772aa72e4e..af3bd0aec538 100644
> --- a/drivers/clk/zynq/clkc.c
> +++ b/drivers/clk/zynq/clkc.c
> @@ -102,9 +102,10 @@ static const char *swdt_ext_clk_input_names[] __initdata = {"swdt_ext_clk"};
>  
>  static void __init zynq_clk_register_fclk(enum zynq_clk fclk,
>  		const char *clk_name, void __iomem *fclk_ctrl_reg,
> -		const char **parents)
> +		const char **parents, int enable)
>  {
>  	struct clk *clk;
> +	u32 enable_reg;
>  	char *mux_name;
>  	char *div0_name;
>  	char *div1_name;
> @@ -147,6 +148,12 @@ static void __init zynq_clk_register_fclk(enum zynq_clk fclk,
>  	clks[fclk] = clk_register_gate(NULL, clk_name,
>  			div1_name, CLK_SET_RATE_PARENT, fclk_gate_reg,
>  			0, CLK_GATE_SET_TO_DISABLE, fclk_gate_lock);
> +	enable_reg = readl(fclk_gate_reg) & 1;
> +	if (enable & !enable_reg) {
> +		if (clk_prepare_enable(clks[fclk]))
> +			pr_warn("%s: FCLK%u enable failed\n", __func__,
> +					fclk - fclk0);
> +	}
>  	kfree(mux_name);
>  	kfree(div0_name);
>  	kfree(div1_name);
> @@ -213,6 +220,7 @@ static void __init zynq_clk_setup(struct device_node *np)
>  	int ret;
>  	struct clk *clk;
>  	char *clk_name;
> +	unsigned int fclk_enable = 0;
>  	const char *clk_output_name[clk_max];
>  	const char *cpu_parents[4];
>  	const char *periph_parents[4];
> @@ -238,6 +246,8 @@ static void __init zynq_clk_setup(struct device_node *np)
>  	periph_parents[2] = clk_output_name[armpll];
>  	periph_parents[3] = clk_output_name[ddrpll];
>  
> +	of_property_read_u32(np, "fclk-enable", &fclk_enable);
> +
>  	/* ps_clk */
>  	ret = of_property_read_u32(np, "ps-clk-frequency", &tmp);
>  	if (ret) {
> @@ -340,10 +350,12 @@ static void __init zynq_clk_setup(struct device_node *np)
>  	clk_prepare_enable(clks[dci]);
>  
>  	/* Peripheral clocks */
> -	for (i = fclk0; i <= fclk3; i++)
> +	for (i = fclk0; i <= fclk3; i++) {
> +		int enable = !!(fclk_enable & BIT(i - fclk0));
>  		zynq_clk_register_fclk(i, clk_output_name[i],
>  				SLCR_FPGA0_CLK_CTRL + 0x10 * (i - fclk0),
> -				periph_parents);
> +				periph_parents, enable);
> +	}
>  
>  	zynq_clk_register_periph_clk(lqspi, 0, clk_output_name[lqspi], NULL,
>  			SLCR_LQSPI_CLK_CTRL, periph_parents, 0);
> -- 
> 1.8.4
> 
> 



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

* Re: [PATCH v2 1/2] clk/zynq/clkc: Add 'fclk-enable' feature
@ 2013-10-28 20:59   ` Sören Brinkmann
  0 siblings, 0 replies; 24+ messages in thread
From: Sören Brinkmann @ 2013-10-28 20:59 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
	Ian Campbell, Rob Landley, Russell King, Mike Turquette,
	Michal Simek
  Cc: linux-kernel, linux-arm-kernel, linux-doc, devicetree

ping?

On Thu, Oct 10, 2013 at 10:10:17AM -0700, Soren Brinkmann wrote:
> In some use cases Zynq's FPGA clocks are used as static clock
> generators for IP in the FPGA part of the SOC for which no Linux driver
> exists and would control those clocks. To avoid automatic
> gating of these clocks in such cases a new property - fclk-enable - is
> added to the clock controller's DT description to accomodate such use
> cases. It's value is a bitmask, where a set bit results in enabling
> the corresponding FCLK through the clkc.
> 
> FPGA clocks are handled following the rules below:
> 
> If an FCLK is not enabled by bootloaders, that FCLK will be disabled in
> Linux. Drivers can enable and control it through the CCF as usual.
> 
> If an FCLK is enabled by bootloaders AND the corresponding bit in the
> 'fclk-enable' DT property is set, that FCLK will be enabled by the clkc,
> resulting in an off by one reference count for that clock. Ensuring it
> will always be running.
> 
> Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
> ---
> v2:
>  - change default value for fclk-enable to '0'
> ---
>  Documentation/devicetree/bindings/clock/zynq-7000.txt |  4 ++++
>  drivers/clk/zynq/clkc.c                               | 18 +++++++++++++++---
>  2 files changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/clock/zynq-7000.txt b/Documentation/devicetree/bindings/clock/zynq-7000.txt
> index d99af878f5d7..11fdd146ec83 100644
> --- a/Documentation/devicetree/bindings/clock/zynq-7000.txt
> +++ b/Documentation/devicetree/bindings/clock/zynq-7000.txt
> @@ -22,6 +22,10 @@ Required properties:
>  Optional properties:
>   - clocks : as described in the clock bindings
>   - clock-names : as described in the clock bindings
> + - fclk-enable : Bit mask to enable FCLKs in cases no proper CCF compatible
> +		 driver is available. Bit [0..3] correspond to FCLK0..FCLK3. The
> +		 corresponding FCLK will only be enabled if it is actually
> +		 running at boot time. (default = 0xf)
>  
>  Clock inputs:
>  The following strings are optional parameters to the 'clock-names' property in
> diff --git a/drivers/clk/zynq/clkc.c b/drivers/clk/zynq/clkc.c
> index 10772aa72e4e..af3bd0aec538 100644
> --- a/drivers/clk/zynq/clkc.c
> +++ b/drivers/clk/zynq/clkc.c
> @@ -102,9 +102,10 @@ static const char *swdt_ext_clk_input_names[] __initdata = {"swdt_ext_clk"};
>  
>  static void __init zynq_clk_register_fclk(enum zynq_clk fclk,
>  		const char *clk_name, void __iomem *fclk_ctrl_reg,
> -		const char **parents)
> +		const char **parents, int enable)
>  {
>  	struct clk *clk;
> +	u32 enable_reg;
>  	char *mux_name;
>  	char *div0_name;
>  	char *div1_name;
> @@ -147,6 +148,12 @@ static void __init zynq_clk_register_fclk(enum zynq_clk fclk,
>  	clks[fclk] = clk_register_gate(NULL, clk_name,
>  			div1_name, CLK_SET_RATE_PARENT, fclk_gate_reg,
>  			0, CLK_GATE_SET_TO_DISABLE, fclk_gate_lock);
> +	enable_reg = readl(fclk_gate_reg) & 1;
> +	if (enable & !enable_reg) {
> +		if (clk_prepare_enable(clks[fclk]))
> +			pr_warn("%s: FCLK%u enable failed\n", __func__,
> +					fclk - fclk0);
> +	}
>  	kfree(mux_name);
>  	kfree(div0_name);
>  	kfree(div1_name);
> @@ -213,6 +220,7 @@ static void __init zynq_clk_setup(struct device_node *np)
>  	int ret;
>  	struct clk *clk;
>  	char *clk_name;
> +	unsigned int fclk_enable = 0;
>  	const char *clk_output_name[clk_max];
>  	const char *cpu_parents[4];
>  	const char *periph_parents[4];
> @@ -238,6 +246,8 @@ static void __init zynq_clk_setup(struct device_node *np)
>  	periph_parents[2] = clk_output_name[armpll];
>  	periph_parents[3] = clk_output_name[ddrpll];
>  
> +	of_property_read_u32(np, "fclk-enable", &fclk_enable);
> +
>  	/* ps_clk */
>  	ret = of_property_read_u32(np, "ps-clk-frequency", &tmp);
>  	if (ret) {
> @@ -340,10 +350,12 @@ static void __init zynq_clk_setup(struct device_node *np)
>  	clk_prepare_enable(clks[dci]);
>  
>  	/* Peripheral clocks */
> -	for (i = fclk0; i <= fclk3; i++)
> +	for (i = fclk0; i <= fclk3; i++) {
> +		int enable = !!(fclk_enable & BIT(i - fclk0));
>  		zynq_clk_register_fclk(i, clk_output_name[i],
>  				SLCR_FPGA0_CLK_CTRL + 0x10 * (i - fclk0),
> -				periph_parents);
> +				periph_parents, enable);
> +	}
>  
>  	zynq_clk_register_periph_clk(lqspi, 0, clk_output_name[lqspi], NULL,
>  			SLCR_LQSPI_CLK_CTRL, periph_parents, 0);
> -- 
> 1.8.4
> 
> 

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

* [PATCH v2 1/2] clk/zynq/clkc: Add 'fclk-enable' feature
@ 2013-10-28 20:59   ` Sören Brinkmann
  0 siblings, 0 replies; 24+ messages in thread
From: Sören Brinkmann @ 2013-10-28 20:59 UTC (permalink / raw)
  To: linux-arm-kernel

ping?

On Thu, Oct 10, 2013 at 10:10:17AM -0700, Soren Brinkmann wrote:
> In some use cases Zynq's FPGA clocks are used as static clock
> generators for IP in the FPGA part of the SOC for which no Linux driver
> exists and would control those clocks. To avoid automatic
> gating of these clocks in such cases a new property - fclk-enable - is
> added to the clock controller's DT description to accomodate such use
> cases. It's value is a bitmask, where a set bit results in enabling
> the corresponding FCLK through the clkc.
> 
> FPGA clocks are handled following the rules below:
> 
> If an FCLK is not enabled by bootloaders, that FCLK will be disabled in
> Linux. Drivers can enable and control it through the CCF as usual.
> 
> If an FCLK is enabled by bootloaders AND the corresponding bit in the
> 'fclk-enable' DT property is set, that FCLK will be enabled by the clkc,
> resulting in an off by one reference count for that clock. Ensuring it
> will always be running.
> 
> Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
> ---
> v2:
>  - change default value for fclk-enable to '0'
> ---
>  Documentation/devicetree/bindings/clock/zynq-7000.txt |  4 ++++
>  drivers/clk/zynq/clkc.c                               | 18 +++++++++++++++---
>  2 files changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/clock/zynq-7000.txt b/Documentation/devicetree/bindings/clock/zynq-7000.txt
> index d99af878f5d7..11fdd146ec83 100644
> --- a/Documentation/devicetree/bindings/clock/zynq-7000.txt
> +++ b/Documentation/devicetree/bindings/clock/zynq-7000.txt
> @@ -22,6 +22,10 @@ Required properties:
>  Optional properties:
>   - clocks : as described in the clock bindings
>   - clock-names : as described in the clock bindings
> + - fclk-enable : Bit mask to enable FCLKs in cases no proper CCF compatible
> +		 driver is available. Bit [0..3] correspond to FCLK0..FCLK3. The
> +		 corresponding FCLK will only be enabled if it is actually
> +		 running at boot time. (default = 0xf)
>  
>  Clock inputs:
>  The following strings are optional parameters to the 'clock-names' property in
> diff --git a/drivers/clk/zynq/clkc.c b/drivers/clk/zynq/clkc.c
> index 10772aa72e4e..af3bd0aec538 100644
> --- a/drivers/clk/zynq/clkc.c
> +++ b/drivers/clk/zynq/clkc.c
> @@ -102,9 +102,10 @@ static const char *swdt_ext_clk_input_names[] __initdata = {"swdt_ext_clk"};
>  
>  static void __init zynq_clk_register_fclk(enum zynq_clk fclk,
>  		const char *clk_name, void __iomem *fclk_ctrl_reg,
> -		const char **parents)
> +		const char **parents, int enable)
>  {
>  	struct clk *clk;
> +	u32 enable_reg;
>  	char *mux_name;
>  	char *div0_name;
>  	char *div1_name;
> @@ -147,6 +148,12 @@ static void __init zynq_clk_register_fclk(enum zynq_clk fclk,
>  	clks[fclk] = clk_register_gate(NULL, clk_name,
>  			div1_name, CLK_SET_RATE_PARENT, fclk_gate_reg,
>  			0, CLK_GATE_SET_TO_DISABLE, fclk_gate_lock);
> +	enable_reg = readl(fclk_gate_reg) & 1;
> +	if (enable & !enable_reg) {
> +		if (clk_prepare_enable(clks[fclk]))
> +			pr_warn("%s: FCLK%u enable failed\n", __func__,
> +					fclk - fclk0);
> +	}
>  	kfree(mux_name);
>  	kfree(div0_name);
>  	kfree(div1_name);
> @@ -213,6 +220,7 @@ static void __init zynq_clk_setup(struct device_node *np)
>  	int ret;
>  	struct clk *clk;
>  	char *clk_name;
> +	unsigned int fclk_enable = 0;
>  	const char *clk_output_name[clk_max];
>  	const char *cpu_parents[4];
>  	const char *periph_parents[4];
> @@ -238,6 +246,8 @@ static void __init zynq_clk_setup(struct device_node *np)
>  	periph_parents[2] = clk_output_name[armpll];
>  	periph_parents[3] = clk_output_name[ddrpll];
>  
> +	of_property_read_u32(np, "fclk-enable", &fclk_enable);
> +
>  	/* ps_clk */
>  	ret = of_property_read_u32(np, "ps-clk-frequency", &tmp);
>  	if (ret) {
> @@ -340,10 +350,12 @@ static void __init zynq_clk_setup(struct device_node *np)
>  	clk_prepare_enable(clks[dci]);
>  
>  	/* Peripheral clocks */
> -	for (i = fclk0; i <= fclk3; i++)
> +	for (i = fclk0; i <= fclk3; i++) {
> +		int enable = !!(fclk_enable & BIT(i - fclk0));
>  		zynq_clk_register_fclk(i, clk_output_name[i],
>  				SLCR_FPGA0_CLK_CTRL + 0x10 * (i - fclk0),
> -				periph_parents);
> +				periph_parents, enable);
> +	}
>  
>  	zynq_clk_register_periph_clk(lqspi, 0, clk_output_name[lqspi], NULL,
>  			SLCR_LQSPI_CLK_CTRL, periph_parents, 0);
> -- 
> 1.8.4
> 
> 

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

* Re: [PATCH v2 1/2] clk/zynq/clkc: Add 'fclk-enable' feature
  2013-10-10 17:10 ` Soren Brinkmann
@ 2013-10-28 21:13   ` Tomasz Figa
  -1 siblings, 0 replies; 24+ messages in thread
From: Tomasz Figa @ 2013-10-28 21:13 UTC (permalink / raw)
  To: Soren Brinkmann
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
	Ian Campbell, Rob Landley, Russell King, Mike Turquette,
	Michal Simek, linux-kernel, linux-arm-kernel, linux-doc,
	devicetree

Hi Soren,

On Thursday 10 of October 2013 10:10:17 Soren Brinkmann wrote:
> In some use cases Zynq's FPGA clocks are used as static clock
> generators for IP in the FPGA part of the SOC for which no Linux driver
> exists and would control those clocks. To avoid automatic
> gating of these clocks in such cases a new property - fclk-enable - is
> added to the clock controller's DT description to accomodate such use
> cases. It's value is a bitmask, where a set bit results in enabling
> the corresponding FCLK through the clkc.
> 
> FPGA clocks are handled following the rules below:
> 
> If an FCLK is not enabled by bootloaders, that FCLK will be disabled in
> Linux. Drivers can enable and control it through the CCF as usual.
> 
> If an FCLK is enabled by bootloaders AND the corresponding bit in the
> 'fclk-enable' DT property is set, that FCLK will be enabled by the clkc,
> resulting in an off by one reference count for that clock. Ensuring it
> will always be running.
> 
> Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
> ---
> v2:
>  - change default value for fclk-enable to '0'
> ---
>  Documentation/devicetree/bindings/clock/zynq-7000.txt |  4 ++++
>  drivers/clk/zynq/clkc.c                               | 18
> +++++++++++++++--- 2 files changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/clock/zynq-7000.txt
> b/Documentation/devicetree/bindings/clock/zynq-7000.txt index
> d99af878f5d7..11fdd146ec83 100644
> --- a/Documentation/devicetree/bindings/clock/zynq-7000.txt
> +++ b/Documentation/devicetree/bindings/clock/zynq-7000.txt
> @@ -22,6 +22,10 @@ Required properties:
>  Optional properties:
>   - clocks : as described in the clock bindings
>   - clock-names : as described in the clock bindings
> + - fclk-enable : Bit mask to enable FCLKs in cases no proper CCF

Since it's a vendor specific property, it should include vendor prefix.

Also CCF is a Linux-specific implementation detail, which DT bindings
should not be involved into. If you really need to implement this using
this way, then at least property description should say something like 
this:

xlnx,fclk-enable : Bit mask of bits of fclk enable register that must
be statically enabled at boot-up time.

However, I wonder why you can't simply define an FPGA block using a single 
node, which would be a consumer to all the fclk clocks you need to enable 
and then make a driver for it that would simply enable all clocks 
specified in clocks property.

Best regards,
Tomasz


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

* [PATCH v2 1/2] clk/zynq/clkc: Add 'fclk-enable' feature
@ 2013-10-28 21:13   ` Tomasz Figa
  0 siblings, 0 replies; 24+ messages in thread
From: Tomasz Figa @ 2013-10-28 21:13 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Soren,

On Thursday 10 of October 2013 10:10:17 Soren Brinkmann wrote:
> In some use cases Zynq's FPGA clocks are used as static clock
> generators for IP in the FPGA part of the SOC for which no Linux driver
> exists and would control those clocks. To avoid automatic
> gating of these clocks in such cases a new property - fclk-enable - is
> added to the clock controller's DT description to accomodate such use
> cases. It's value is a bitmask, where a set bit results in enabling
> the corresponding FCLK through the clkc.
> 
> FPGA clocks are handled following the rules below:
> 
> If an FCLK is not enabled by bootloaders, that FCLK will be disabled in
> Linux. Drivers can enable and control it through the CCF as usual.
> 
> If an FCLK is enabled by bootloaders AND the corresponding bit in the
> 'fclk-enable' DT property is set, that FCLK will be enabled by the clkc,
> resulting in an off by one reference count for that clock. Ensuring it
> will always be running.
> 
> Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
> ---
> v2:
>  - change default value for fclk-enable to '0'
> ---
>  Documentation/devicetree/bindings/clock/zynq-7000.txt |  4 ++++
>  drivers/clk/zynq/clkc.c                               | 18
> +++++++++++++++--- 2 files changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/clock/zynq-7000.txt
> b/Documentation/devicetree/bindings/clock/zynq-7000.txt index
> d99af878f5d7..11fdd146ec83 100644
> --- a/Documentation/devicetree/bindings/clock/zynq-7000.txt
> +++ b/Documentation/devicetree/bindings/clock/zynq-7000.txt
> @@ -22,6 +22,10 @@ Required properties:
>  Optional properties:
>   - clocks : as described in the clock bindings
>   - clock-names : as described in the clock bindings
> + - fclk-enable : Bit mask to enable FCLKs in cases no proper CCF

Since it's a vendor specific property, it should include vendor prefix.

Also CCF is a Linux-specific implementation detail, which DT bindings
should not be involved into. If you really need to implement this using
this way, then at least property description should say something like 
this:

xlnx,fclk-enable : Bit mask of bits of fclk enable register that must
be statically enabled at boot-up time.

However, I wonder why you can't simply define an FPGA block using a single 
node, which would be a consumer to all the fclk clocks you need to enable 
and then make a driver for it that would simply enable all clocks 
specified in clocks property.

Best regards,
Tomasz

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

* Re: [PATCH v2 1/2] clk/zynq/clkc: Add 'fclk-enable' feature
  2013-10-28 21:13   ` Tomasz Figa
  (?)
@ 2013-10-28 21:43     ` Sören Brinkmann
  -1 siblings, 0 replies; 24+ messages in thread
From: Sören Brinkmann @ 2013-10-28 21:43 UTC (permalink / raw)
  To: Tomasz Figa
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
	Ian Campbell, Rob Landley, Russell King, Mike Turquette,
	Michal Simek, linux-kernel, linux-arm-kernel, linux-doc,
	devicetree

On Mon, Oct 28, 2013 at 10:13:28PM +0100, Tomasz Figa wrote:
> Hi Soren,
> 
> On Thursday 10 of October 2013 10:10:17 Soren Brinkmann wrote:
> > In some use cases Zynq's FPGA clocks are used as static clock
> > generators for IP in the FPGA part of the SOC for which no Linux driver
> > exists and would control those clocks. To avoid automatic
> > gating of these clocks in such cases a new property - fclk-enable - is
> > added to the clock controller's DT description to accomodate such use
> > cases. It's value is a bitmask, where a set bit results in enabling
> > the corresponding FCLK through the clkc.
> > 
> > FPGA clocks are handled following the rules below:
> > 
> > If an FCLK is not enabled by bootloaders, that FCLK will be disabled in
> > Linux. Drivers can enable and control it through the CCF as usual.
> > 
> > If an FCLK is enabled by bootloaders AND the corresponding bit in the
> > 'fclk-enable' DT property is set, that FCLK will be enabled by the clkc,
> > resulting in an off by one reference count for that clock. Ensuring it
> > will always be running.
> > 
> > Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
> > ---
> > v2:
> >  - change default value for fclk-enable to '0'
> > ---
> >  Documentation/devicetree/bindings/clock/zynq-7000.txt |  4 ++++
> >  drivers/clk/zynq/clkc.c                               | 18
> > +++++++++++++++--- 2 files changed, 19 insertions(+), 3 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/clock/zynq-7000.txt
> > b/Documentation/devicetree/bindings/clock/zynq-7000.txt index
> > d99af878f5d7..11fdd146ec83 100644
> > --- a/Documentation/devicetree/bindings/clock/zynq-7000.txt
> > +++ b/Documentation/devicetree/bindings/clock/zynq-7000.txt
> > @@ -22,6 +22,10 @@ Required properties:
> >  Optional properties:
> >   - clocks : as described in the clock bindings
> >   - clock-names : as described in the clock bindings
> > + - fclk-enable : Bit mask to enable FCLKs in cases no proper CCF
> 
> Since it's a vendor specific property, it should include vendor prefix.
The whole driver is vendor specific. Should there really be another
prefix for that property?

> 
> Also CCF is a Linux-specific implementation detail, which DT bindings
> should not be involved into. If you really need to implement this using
> this way, then at least property description should say something like 
> this:
> 
> xlnx,fclk-enable : Bit mask of bits of fclk enable register that must
> be statically enabled at boot-up time.
Fair enough. I'll change the description

> 
> However, I wonder why you can't simply define an FPGA block using a single 
> node, which would be a consumer to all the fclk clocks you need to enable 
> and then make a driver for it that would simply enable all clocks 
> specified in clocks property.
Well, then we'd have a dummy driver that wouldn't fit into any subsystem
and wouldn't do anything but enabling clocks. Seems much easier to
handle it in this driver. Especially, since I hope that this is just a
workaround and that the majority of use cases involves drivers for their
soft-IP that simply uses the CCF.

	Sören



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

* Re: [PATCH v2 1/2] clk/zynq/clkc: Add 'fclk-enable' feature
@ 2013-10-28 21:43     ` Sören Brinkmann
  0 siblings, 0 replies; 24+ messages in thread
From: Sören Brinkmann @ 2013-10-28 21:43 UTC (permalink / raw)
  To: Tomasz Figa
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
	Ian Campbell, Rob Landley, Russell King, Mike Turquette,
	Michal Simek, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-doc-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA

On Mon, Oct 28, 2013 at 10:13:28PM +0100, Tomasz Figa wrote:
> Hi Soren,
> 
> On Thursday 10 of October 2013 10:10:17 Soren Brinkmann wrote:
> > In some use cases Zynq's FPGA clocks are used as static clock
> > generators for IP in the FPGA part of the SOC for which no Linux driver
> > exists and would control those clocks. To avoid automatic
> > gating of these clocks in such cases a new property - fclk-enable - is
> > added to the clock controller's DT description to accomodate such use
> > cases. It's value is a bitmask, where a set bit results in enabling
> > the corresponding FCLK through the clkc.
> > 
> > FPGA clocks are handled following the rules below:
> > 
> > If an FCLK is not enabled by bootloaders, that FCLK will be disabled in
> > Linux. Drivers can enable and control it through the CCF as usual.
> > 
> > If an FCLK is enabled by bootloaders AND the corresponding bit in the
> > 'fclk-enable' DT property is set, that FCLK will be enabled by the clkc,
> > resulting in an off by one reference count for that clock. Ensuring it
> > will always be running.
> > 
> > Signed-off-by: Soren Brinkmann <soren.brinkmann-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
> > ---
> > v2:
> >  - change default value for fclk-enable to '0'
> > ---
> >  Documentation/devicetree/bindings/clock/zynq-7000.txt |  4 ++++
> >  drivers/clk/zynq/clkc.c                               | 18
> > +++++++++++++++--- 2 files changed, 19 insertions(+), 3 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/clock/zynq-7000.txt
> > b/Documentation/devicetree/bindings/clock/zynq-7000.txt index
> > d99af878f5d7..11fdd146ec83 100644
> > --- a/Documentation/devicetree/bindings/clock/zynq-7000.txt
> > +++ b/Documentation/devicetree/bindings/clock/zynq-7000.txt
> > @@ -22,6 +22,10 @@ Required properties:
> >  Optional properties:
> >   - clocks : as described in the clock bindings
> >   - clock-names : as described in the clock bindings
> > + - fclk-enable : Bit mask to enable FCLKs in cases no proper CCF
> 
> Since it's a vendor specific property, it should include vendor prefix.
The whole driver is vendor specific. Should there really be another
prefix for that property?

> 
> Also CCF is a Linux-specific implementation detail, which DT bindings
> should not be involved into. If you really need to implement this using
> this way, then at least property description should say something like 
> this:
> 
> xlnx,fclk-enable : Bit mask of bits of fclk enable register that must
> be statically enabled at boot-up time.
Fair enough. I'll change the description

> 
> However, I wonder why you can't simply define an FPGA block using a single 
> node, which would be a consumer to all the fclk clocks you need to enable 
> and then make a driver for it that would simply enable all clocks 
> specified in clocks property.
Well, then we'd have a dummy driver that wouldn't fit into any subsystem
and wouldn't do anything but enabling clocks. Seems much easier to
handle it in this driver. Especially, since I hope that this is just a
workaround and that the majority of use cases involves drivers for their
soft-IP that simply uses the CCF.

	Sören


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

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

* [PATCH v2 1/2] clk/zynq/clkc: Add 'fclk-enable' feature
@ 2013-10-28 21:43     ` Sören Brinkmann
  0 siblings, 0 replies; 24+ messages in thread
From: Sören Brinkmann @ 2013-10-28 21:43 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Oct 28, 2013 at 10:13:28PM +0100, Tomasz Figa wrote:
> Hi Soren,
> 
> On Thursday 10 of October 2013 10:10:17 Soren Brinkmann wrote:
> > In some use cases Zynq's FPGA clocks are used as static clock
> > generators for IP in the FPGA part of the SOC for which no Linux driver
> > exists and would control those clocks. To avoid automatic
> > gating of these clocks in such cases a new property - fclk-enable - is
> > added to the clock controller's DT description to accomodate such use
> > cases. It's value is a bitmask, where a set bit results in enabling
> > the corresponding FCLK through the clkc.
> > 
> > FPGA clocks are handled following the rules below:
> > 
> > If an FCLK is not enabled by bootloaders, that FCLK will be disabled in
> > Linux. Drivers can enable and control it through the CCF as usual.
> > 
> > If an FCLK is enabled by bootloaders AND the corresponding bit in the
> > 'fclk-enable' DT property is set, that FCLK will be enabled by the clkc,
> > resulting in an off by one reference count for that clock. Ensuring it
> > will always be running.
> > 
> > Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
> > ---
> > v2:
> >  - change default value for fclk-enable to '0'
> > ---
> >  Documentation/devicetree/bindings/clock/zynq-7000.txt |  4 ++++
> >  drivers/clk/zynq/clkc.c                               | 18
> > +++++++++++++++--- 2 files changed, 19 insertions(+), 3 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/clock/zynq-7000.txt
> > b/Documentation/devicetree/bindings/clock/zynq-7000.txt index
> > d99af878f5d7..11fdd146ec83 100644
> > --- a/Documentation/devicetree/bindings/clock/zynq-7000.txt
> > +++ b/Documentation/devicetree/bindings/clock/zynq-7000.txt
> > @@ -22,6 +22,10 @@ Required properties:
> >  Optional properties:
> >   - clocks : as described in the clock bindings
> >   - clock-names : as described in the clock bindings
> > + - fclk-enable : Bit mask to enable FCLKs in cases no proper CCF
> 
> Since it's a vendor specific property, it should include vendor prefix.
The whole driver is vendor specific. Should there really be another
prefix for that property?

> 
> Also CCF is a Linux-specific implementation detail, which DT bindings
> should not be involved into. If you really need to implement this using
> this way, then at least property description should say something like 
> this:
> 
> xlnx,fclk-enable : Bit mask of bits of fclk enable register that must
> be statically enabled at boot-up time.
Fair enough. I'll change the description

> 
> However, I wonder why you can't simply define an FPGA block using a single 
> node, which would be a consumer to all the fclk clocks you need to enable 
> and then make a driver for it that would simply enable all clocks 
> specified in clocks property.
Well, then we'd have a dummy driver that wouldn't fit into any subsystem
and wouldn't do anything but enabling clocks. Seems much easier to
handle it in this driver. Especially, since I hope that this is just a
workaround and that the majority of use cases involves drivers for their
soft-IP that simply uses the CCF.

	S?ren

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

* Re: [PATCH v2 1/2] clk/zynq/clkc: Add 'fclk-enable' feature
  2013-10-28 21:43     ` Sören Brinkmann
@ 2013-10-28 22:17       ` Tomasz Figa
  -1 siblings, 0 replies; 24+ messages in thread
From: Tomasz Figa @ 2013-10-28 22:17 UTC (permalink / raw)
  To: Sören Brinkmann
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
	Ian Campbell, Rob Landley, Russell King, Mike Turquette,
	Michal Simek, linux-kernel, linux-arm-kernel, linux-doc,
	devicetree

On Monday 28 of October 2013 14:43:35 Sören Brinkmann wrote:
> On Mon, Oct 28, 2013 at 10:13:28PM +0100, Tomasz Figa wrote:
> > Hi Soren,
> > 
> > On Thursday 10 of October 2013 10:10:17 Soren Brinkmann wrote:
> > > In some use cases Zynq's FPGA clocks are used as static clock
> > > generators for IP in the FPGA part of the SOC for which no Linux
> > > driver
> > > exists and would control those clocks. To avoid automatic
> > > gating of these clocks in such cases a new property - fclk-enable -
> > > is
> > > added to the clock controller's DT description to accomodate such
> > > use
> > > cases. It's value is a bitmask, where a set bit results in enabling
> > > the corresponding FCLK through the clkc.
> > > 
> > > FPGA clocks are handled following the rules below:
> > > 
> > > If an FCLK is not enabled by bootloaders, that FCLK will be disabled
> > > in
> > > Linux. Drivers can enable and control it through the CCF as usual.
> > > 
> > > If an FCLK is enabled by bootloaders AND the corresponding bit in
> > > the
> > > 'fclk-enable' DT property is set, that FCLK will be enabled by the
> > > clkc, resulting in an off by one reference count for that clock.
> > > Ensuring it will always be running.
> > > 
> > > Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
> > > ---
> > > 
> > > v2:
> > >  - change default value for fclk-enable to '0'
> > > 
> > > ---
> > > 
> > >  Documentation/devicetree/bindings/clock/zynq-7000.txt |  4 ++++
> > >  drivers/clk/zynq/clkc.c                               | 18
> > > 
> > > +++++++++++++++--- 2 files changed, 19 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/clock/zynq-7000.txt
> > > b/Documentation/devicetree/bindings/clock/zynq-7000.txt index
> > > d99af878f5d7..11fdd146ec83 100644
> > > --- a/Documentation/devicetree/bindings/clock/zynq-7000.txt
> > > +++ b/Documentation/devicetree/bindings/clock/zynq-7000.txt
> > > 
> > > @@ -22,6 +22,10 @@ Required properties:
> > >  Optional properties:
> > >   - clocks : as described in the clock bindings
> > >   - clock-names : as described in the clock bindings
> > > 
> > > + - fclk-enable : Bit mask to enable FCLKs in cases no proper CCF
> > 
> > Since it's a vendor specific property, it should include vendor
> > prefix.
> 
> The whole driver is vendor specific. Should there really be another
> prefix for that property?

Yes. If a property is introduced just for use by this particular driver 
then it must be prepended by a vendor prefix. That's a general rule.

> > Also CCF is a Linux-specific implementation detail, which DT bindings
> > should not be involved into. If you really need to implement this
> > using
> > this way, then at least property description should say something like
> > this:
> > 
> > xlnx,fclk-enable : Bit mask of bits of fclk enable register that must
> > be statically enabled at boot-up time.
> 
> Fair enough. I'll change the description
> 
> > However, I wonder why you can't simply define an FPGA block using a
> > single node, which would be a consumer to all the fclk clocks you
> > need to enable and then make a driver for it that would simply enable
> > all clocks specified in clocks property.
> 
> Well, then we'd have a dummy driver that wouldn't fit into any subsystem
> and wouldn't do anything but enabling clocks. Seems much easier to
> handle it in this driver. Especially, since I hope that this is just a
> workaround and that the majority of use cases involves drivers for
> their soft-IP that simply uses the CCF.

Hmm, I'm not really convinced, but well, let's say that I'm fine with your 
proposed solution, unless someone else complains.

Best regards,
Tomasz


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

* [PATCH v2 1/2] clk/zynq/clkc: Add 'fclk-enable' feature
@ 2013-10-28 22:17       ` Tomasz Figa
  0 siblings, 0 replies; 24+ messages in thread
From: Tomasz Figa @ 2013-10-28 22:17 UTC (permalink / raw)
  To: linux-arm-kernel

On Monday 28 of October 2013 14:43:35 S?ren Brinkmann wrote:
> On Mon, Oct 28, 2013 at 10:13:28PM +0100, Tomasz Figa wrote:
> > Hi Soren,
> > 
> > On Thursday 10 of October 2013 10:10:17 Soren Brinkmann wrote:
> > > In some use cases Zynq's FPGA clocks are used as static clock
> > > generators for IP in the FPGA part of the SOC for which no Linux
> > > driver
> > > exists and would control those clocks. To avoid automatic
> > > gating of these clocks in such cases a new property - fclk-enable -
> > > is
> > > added to the clock controller's DT description to accomodate such
> > > use
> > > cases. It's value is a bitmask, where a set bit results in enabling
> > > the corresponding FCLK through the clkc.
> > > 
> > > FPGA clocks are handled following the rules below:
> > > 
> > > If an FCLK is not enabled by bootloaders, that FCLK will be disabled
> > > in
> > > Linux. Drivers can enable and control it through the CCF as usual.
> > > 
> > > If an FCLK is enabled by bootloaders AND the corresponding bit in
> > > the
> > > 'fclk-enable' DT property is set, that FCLK will be enabled by the
> > > clkc, resulting in an off by one reference count for that clock.
> > > Ensuring it will always be running.
> > > 
> > > Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
> > > ---
> > > 
> > > v2:
> > >  - change default value for fclk-enable to '0'
> > > 
> > > ---
> > > 
> > >  Documentation/devicetree/bindings/clock/zynq-7000.txt |  4 ++++
> > >  drivers/clk/zynq/clkc.c                               | 18
> > > 
> > > +++++++++++++++--- 2 files changed, 19 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/clock/zynq-7000.txt
> > > b/Documentation/devicetree/bindings/clock/zynq-7000.txt index
> > > d99af878f5d7..11fdd146ec83 100644
> > > --- a/Documentation/devicetree/bindings/clock/zynq-7000.txt
> > > +++ b/Documentation/devicetree/bindings/clock/zynq-7000.txt
> > > 
> > > @@ -22,6 +22,10 @@ Required properties:
> > >  Optional properties:
> > >   - clocks : as described in the clock bindings
> > >   - clock-names : as described in the clock bindings
> > > 
> > > + - fclk-enable : Bit mask to enable FCLKs in cases no proper CCF
> > 
> > Since it's a vendor specific property, it should include vendor
> > prefix.
> 
> The whole driver is vendor specific. Should there really be another
> prefix for that property?

Yes. If a property is introduced just for use by this particular driver 
then it must be prepended by a vendor prefix. That's a general rule.

> > Also CCF is a Linux-specific implementation detail, which DT bindings
> > should not be involved into. If you really need to implement this
> > using
> > this way, then at least property description should say something like
> > this:
> > 
> > xlnx,fclk-enable : Bit mask of bits of fclk enable register that must
> > be statically enabled at boot-up time.
> 
> Fair enough. I'll change the description
> 
> > However, I wonder why you can't simply define an FPGA block using a
> > single node, which would be a consumer to all the fclk clocks you
> > need to enable and then make a driver for it that would simply enable
> > all clocks specified in clocks property.
> 
> Well, then we'd have a dummy driver that wouldn't fit into any subsystem
> and wouldn't do anything but enabling clocks. Seems much easier to
> handle it in this driver. Especially, since I hope that this is just a
> workaround and that the majority of use cases involves drivers for
> their soft-IP that simply uses the CCF.

Hmm, I'm not really convinced, but well, let's say that I'm fine with your 
proposed solution, unless someone else complains.

Best regards,
Tomasz

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

* Re: [PATCH v2 1/2] clk/zynq/clkc: Add 'fclk-enable' feature
  2013-10-28 22:17       ` Tomasz Figa
@ 2013-10-29  8:26         ` Kumar Gala
  -1 siblings, 0 replies; 24+ messages in thread
From: Kumar Gala @ 2013-10-29  8:26 UTC (permalink / raw)
  To: Tomasz Figa
  Cc: Sören Brinkmann, Rob Herring, Pawel Moll, Mark Rutland,
	Stephen Warren, Ian Campbell, Rob Landley, Russell King,
	Mike Turquette, Michal Simek, linux-kernel, linux-arm-kernel,
	linux-doc, devicetree


On Oct 28, 2013, at 5:17 PM, Tomasz Figa wrote:

>>>> diff --git a/Documentation/devicetree/bindings/clock/zynq-7000.txt
>>>> b/Documentation/devicetree/bindings/clock/zynq-7000.txt index
>>>> d99af878f5d7..11fdd146ec83 100644
>>>> --- a/Documentation/devicetree/bindings/clock/zynq-7000.txt
>>>> +++ b/Documentation/devicetree/bindings/clock/zynq-7000.txt
>>>> 
>>>> @@ -22,6 +22,10 @@ Required properties:
>>>> Optional properties:
>>>>  - clocks : as described in the clock bindings
>>>>  - clock-names : as described in the clock bindings
>>>> 
>>>> + - fclk-enable : Bit mask to enable FCLKs in cases no proper CCF
>>> 
>>> Since it's a vendor specific property, it should include vendor
>>> prefix.
>> 
>> The whole driver is vendor specific. Should there really be another
>> prefix for that property?
> 
> Yes. If a property is introduced just for use by this particular driver 
> then it must be prepended by a vendor prefix. That's a general rule.

Most all nodes are vendor specific by definition ;).

- k

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation


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

* [PATCH v2 1/2] clk/zynq/clkc: Add 'fclk-enable' feature
@ 2013-10-29  8:26         ` Kumar Gala
  0 siblings, 0 replies; 24+ messages in thread
From: Kumar Gala @ 2013-10-29  8:26 UTC (permalink / raw)
  To: linux-arm-kernel


On Oct 28, 2013, at 5:17 PM, Tomasz Figa wrote:

>>>> diff --git a/Documentation/devicetree/bindings/clock/zynq-7000.txt
>>>> b/Documentation/devicetree/bindings/clock/zynq-7000.txt index
>>>> d99af878f5d7..11fdd146ec83 100644
>>>> --- a/Documentation/devicetree/bindings/clock/zynq-7000.txt
>>>> +++ b/Documentation/devicetree/bindings/clock/zynq-7000.txt
>>>> 
>>>> @@ -22,6 +22,10 @@ Required properties:
>>>> Optional properties:
>>>>  - clocks : as described in the clock bindings
>>>>  - clock-names : as described in the clock bindings
>>>> 
>>>> + - fclk-enable : Bit mask to enable FCLKs in cases no proper CCF
>>> 
>>> Since it's a vendor specific property, it should include vendor
>>> prefix.
>> 
>> The whole driver is vendor specific. Should there really be another
>> prefix for that property?
> 
> Yes. If a property is introduced just for use by this particular driver 
> then it must be prepended by a vendor prefix. That's a general rule.

Most all nodes are vendor specific by definition ;).

- k

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

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

* Re: [PATCH v2 1/2] clk/zynq/clkc: Add 'fclk-enable' feature
  2013-10-29  8:26         ` Kumar Gala
@ 2013-10-29 13:04           ` Michal Simek
  -1 siblings, 0 replies; 24+ messages in thread
From: Michal Simek @ 2013-10-29 13:04 UTC (permalink / raw)
  To: Kumar Gala
  Cc: Tomasz Figa, Sören Brinkmann, Rob Herring, Pawel Moll,
	Mark Rutland, Stephen Warren, Ian Campbell, Rob Landley,
	Russell King, Mike Turquette, Michal Simek, linux-kernel,
	linux-arm-kernel, linux-doc, devicetree

On 10/29/2013 09:26 AM, Kumar Gala wrote:
> 
> On Oct 28, 2013, at 5:17 PM, Tomasz Figa wrote:
> 
>>>>> diff --git a/Documentation/devicetree/bindings/clock/zynq-7000.txt
>>>>> b/Documentation/devicetree/bindings/clock/zynq-7000.txt index
>>>>> d99af878f5d7..11fdd146ec83 100644
>>>>> --- a/Documentation/devicetree/bindings/clock/zynq-7000.txt
>>>>> +++ b/Documentation/devicetree/bindings/clock/zynq-7000.txt
>>>>>
>>>>> @@ -22,6 +22,10 @@ Required properties:
>>>>> Optional properties:
>>>>>  - clocks : as described in the clock bindings
>>>>>  - clock-names : as described in the clock bindings
>>>>>
>>>>> + - fclk-enable : Bit mask to enable FCLKs in cases no proper CCF
>>>>
>>>> Since it's a vendor specific property, it should include vendor
>>>> prefix.
>>>
>>> The whole driver is vendor specific. Should there really be another
>>> prefix for that property?
>>
>> Yes. If a property is introduced just for use by this particular driver 
>> then it must be prepended by a vendor prefix. That's a general rule.
> 
> Most all nodes are vendor specific by definition ;).

Is this really generic rule? I haven't seen/heard any point regarding this on KS.

I don't think we should use prefix here. It is xilinx specific option
but there shouldn't be any problem to use fclk-enable without any prefix.
Because it means we have to also rename ps-clk-frequency.

We are using xlnx prefix for properties which are autogenerated from design tools
which is not even this case.

Thanks,
Michal



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

* [PATCH v2 1/2] clk/zynq/clkc: Add 'fclk-enable' feature
@ 2013-10-29 13:04           ` Michal Simek
  0 siblings, 0 replies; 24+ messages in thread
From: Michal Simek @ 2013-10-29 13:04 UTC (permalink / raw)
  To: linux-arm-kernel

On 10/29/2013 09:26 AM, Kumar Gala wrote:
> 
> On Oct 28, 2013, at 5:17 PM, Tomasz Figa wrote:
> 
>>>>> diff --git a/Documentation/devicetree/bindings/clock/zynq-7000.txt
>>>>> b/Documentation/devicetree/bindings/clock/zynq-7000.txt index
>>>>> d99af878f5d7..11fdd146ec83 100644
>>>>> --- a/Documentation/devicetree/bindings/clock/zynq-7000.txt
>>>>> +++ b/Documentation/devicetree/bindings/clock/zynq-7000.txt
>>>>>
>>>>> @@ -22,6 +22,10 @@ Required properties:
>>>>> Optional properties:
>>>>>  - clocks : as described in the clock bindings
>>>>>  - clock-names : as described in the clock bindings
>>>>>
>>>>> + - fclk-enable : Bit mask to enable FCLKs in cases no proper CCF
>>>>
>>>> Since it's a vendor specific property, it should include vendor
>>>> prefix.
>>>
>>> The whole driver is vendor specific. Should there really be another
>>> prefix for that property?
>>
>> Yes. If a property is introduced just for use by this particular driver 
>> then it must be prepended by a vendor prefix. That's a general rule.
> 
> Most all nodes are vendor specific by definition ;).

Is this really generic rule? I haven't seen/heard any point regarding this on KS.

I don't think we should use prefix here. It is xilinx specific option
but there shouldn't be any problem to use fclk-enable without any prefix.
Because it means we have to also rename ps-clk-frequency.

We are using xlnx prefix for properties which are autogenerated from design tools
which is not even this case.

Thanks,
Michal

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

* Re: [PATCH v2 1/2] clk/zynq/clkc: Add 'fclk-enable' feature
  2013-10-29 13:04           ` Michal Simek
  (?)
@ 2013-10-30 18:44             ` Sören Brinkmann
  -1 siblings, 0 replies; 24+ messages in thread
From: Sören Brinkmann @ 2013-10-30 18:44 UTC (permalink / raw)
  To: Michal Simek
  Cc: Kumar Gala, Tomasz Figa, Rob Herring, Pawel Moll, Mark Rutland,
	Stephen Warren, Ian Campbell, Rob Landley, Russell King,
	Mike Turquette, linux-kernel, linux-arm-kernel, linux-doc,
	devicetree

On Tue, Oct 29, 2013 at 02:04:06PM +0100, Michal Simek wrote:
> On 10/29/2013 09:26 AM, Kumar Gala wrote:
> > 
> > On Oct 28, 2013, at 5:17 PM, Tomasz Figa wrote:
> > 
> >>>>> diff --git a/Documentation/devicetree/bindings/clock/zynq-7000.txt
> >>>>> b/Documentation/devicetree/bindings/clock/zynq-7000.txt index
> >>>>> d99af878f5d7..11fdd146ec83 100644
> >>>>> --- a/Documentation/devicetree/bindings/clock/zynq-7000.txt
> >>>>> +++ b/Documentation/devicetree/bindings/clock/zynq-7000.txt
> >>>>>
> >>>>> @@ -22,6 +22,10 @@ Required properties:
> >>>>> Optional properties:
> >>>>>  - clocks : as described in the clock bindings
> >>>>>  - clock-names : as described in the clock bindings
> >>>>>
> >>>>> + - fclk-enable : Bit mask to enable FCLKs in cases no proper CCF
> >>>>
> >>>> Since it's a vendor specific property, it should include vendor
> >>>> prefix.
> >>>
> >>> The whole driver is vendor specific. Should there really be another
> >>> prefix for that property?
> >>
> >> Yes. If a property is introduced just for use by this particular driver 
> >> then it must be prepended by a vendor prefix. That's a general rule.
> > 
> > Most all nodes are vendor specific by definition ;).
> 
> Is this really generic rule? I haven't seen/heard any point regarding this on KS.
> 
> I don't think we should use prefix here. It is xilinx specific option
> but there shouldn't be any problem to use fclk-enable without any prefix.
> Because it means we have to also rename ps-clk-frequency.
> 
> We are using xlnx prefix for properties which are autogenerated from design tools
> which is not even this case.

So, what is the final call on this? Respin this and changing the prop name,
or leaving it as is?

	Thanks,
	Sören



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

* Re: [PATCH v2 1/2] clk/zynq/clkc: Add 'fclk-enable' feature
@ 2013-10-30 18:44             ` Sören Brinkmann
  0 siblings, 0 replies; 24+ messages in thread
From: Sören Brinkmann @ 2013-10-30 18:44 UTC (permalink / raw)
  To: Michal Simek
  Cc: Kumar Gala, Tomasz Figa, Rob Herring, Pawel Moll, Mark Rutland,
	Stephen Warren, Ian Campbell, Rob Landley, Russell King,
	Mike Turquette, linux-kernel, linux-arm-kernel, linux-doc,
	devicetree

On Tue, Oct 29, 2013 at 02:04:06PM +0100, Michal Simek wrote:
> On 10/29/2013 09:26 AM, Kumar Gala wrote:
> > 
> > On Oct 28, 2013, at 5:17 PM, Tomasz Figa wrote:
> > 
> >>>>> diff --git a/Documentation/devicetree/bindings/clock/zynq-7000.txt
> >>>>> b/Documentation/devicetree/bindings/clock/zynq-7000.txt index
> >>>>> d99af878f5d7..11fdd146ec83 100644
> >>>>> --- a/Documentation/devicetree/bindings/clock/zynq-7000.txt
> >>>>> +++ b/Documentation/devicetree/bindings/clock/zynq-7000.txt
> >>>>>
> >>>>> @@ -22,6 +22,10 @@ Required properties:
> >>>>> Optional properties:
> >>>>>  - clocks : as described in the clock bindings
> >>>>>  - clock-names : as described in the clock bindings
> >>>>>
> >>>>> + - fclk-enable : Bit mask to enable FCLKs in cases no proper CCF
> >>>>
> >>>> Since it's a vendor specific property, it should include vendor
> >>>> prefix.
> >>>
> >>> The whole driver is vendor specific. Should there really be another
> >>> prefix for that property?
> >>
> >> Yes. If a property is introduced just for use by this particular driver 
> >> then it must be prepended by a vendor prefix. That's a general rule.
> > 
> > Most all nodes are vendor specific by definition ;).
> 
> Is this really generic rule? I haven't seen/heard any point regarding this on KS.
> 
> I don't think we should use prefix here. It is xilinx specific option
> but there shouldn't be any problem to use fclk-enable without any prefix.
> Because it means we have to also rename ps-clk-frequency.
> 
> We are using xlnx prefix for properties which are autogenerated from design tools
> which is not even this case.

So, what is the final call on this? Respin this and changing the prop name,
or leaving it as is?

	Thanks,
	Sören

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

* [PATCH v2 1/2] clk/zynq/clkc: Add 'fclk-enable' feature
@ 2013-10-30 18:44             ` Sören Brinkmann
  0 siblings, 0 replies; 24+ messages in thread
From: Sören Brinkmann @ 2013-10-30 18:44 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Oct 29, 2013 at 02:04:06PM +0100, Michal Simek wrote:
> On 10/29/2013 09:26 AM, Kumar Gala wrote:
> > 
> > On Oct 28, 2013, at 5:17 PM, Tomasz Figa wrote:
> > 
> >>>>> diff --git a/Documentation/devicetree/bindings/clock/zynq-7000.txt
> >>>>> b/Documentation/devicetree/bindings/clock/zynq-7000.txt index
> >>>>> d99af878f5d7..11fdd146ec83 100644
> >>>>> --- a/Documentation/devicetree/bindings/clock/zynq-7000.txt
> >>>>> +++ b/Documentation/devicetree/bindings/clock/zynq-7000.txt
> >>>>>
> >>>>> @@ -22,6 +22,10 @@ Required properties:
> >>>>> Optional properties:
> >>>>>  - clocks : as described in the clock bindings
> >>>>>  - clock-names : as described in the clock bindings
> >>>>>
> >>>>> + - fclk-enable : Bit mask to enable FCLKs in cases no proper CCF
> >>>>
> >>>> Since it's a vendor specific property, it should include vendor
> >>>> prefix.
> >>>
> >>> The whole driver is vendor specific. Should there really be another
> >>> prefix for that property?
> >>
> >> Yes. If a property is introduced just for use by this particular driver 
> >> then it must be prepended by a vendor prefix. That's a general rule.
> > 
> > Most all nodes are vendor specific by definition ;).
> 
> Is this really generic rule? I haven't seen/heard any point regarding this on KS.
> 
> I don't think we should use prefix here. It is xilinx specific option
> but there shouldn't be any problem to use fclk-enable without any prefix.
> Because it means we have to also rename ps-clk-frequency.
> 
> We are using xlnx prefix for properties which are autogenerated from design tools
> which is not even this case.

So, what is the final call on this? Respin this and changing the prop name,
or leaving it as is?

	Thanks,
	S?ren

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

end of thread, other threads:[~2013-10-30 18:44 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-10 17:10 [PATCH v2 1/2] clk/zynq/clkc: Add 'fclk-enable' feature Soren Brinkmann
2013-10-10 17:10 ` Soren Brinkmann
2013-10-10 17:10 ` Soren Brinkmann
2013-10-10 17:10 ` [PATCH v2 2/2] arm: dt: zynq: Add fclk-enable property to clkc node Soren Brinkmann
2013-10-10 17:10   ` Soren Brinkmann
2013-10-14  9:04 ` [PATCH v2 1/2] clk/zynq/clkc: Add 'fclk-enable' feature Michal Simek
2013-10-14  9:04   ` Michal Simek
2013-10-28 20:59 ` Sören Brinkmann
2013-10-28 20:59   ` Sören Brinkmann
2013-10-28 20:59   ` Sören Brinkmann
2013-10-28 21:13 ` Tomasz Figa
2013-10-28 21:13   ` Tomasz Figa
2013-10-28 21:43   ` Sören Brinkmann
2013-10-28 21:43     ` Sören Brinkmann
2013-10-28 21:43     ` Sören Brinkmann
2013-10-28 22:17     ` Tomasz Figa
2013-10-28 22:17       ` Tomasz Figa
2013-10-29  8:26       ` Kumar Gala
2013-10-29  8:26         ` Kumar Gala
2013-10-29 13:04         ` Michal Simek
2013-10-29 13:04           ` Michal Simek
2013-10-30 18:44           ` Sören Brinkmann
2013-10-30 18:44             ` Sören Brinkmann
2013-10-30 18:44             ` Sören Brinkmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.