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

Groups > linux.kernel > #1470835 > unrolled thread

[PATCH 24/26] dt-bindings: Document img,boston-clock binding

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 24/26] dt-bindings: Document img,boston-clock binding Paul Burton <paul.burton@imgtec.com> - 2016-08-26 17:50 +0200
    Re: [PATCH 24/26] dt-bindings: Document img,boston-clock binding Stephen Boyd <sboyd@codeaurora.org> - 2016-08-26 19:50 +0200
      Re: [PATCH 24/26] dt-bindings: Document img,boston-clock binding Paul Burton <paul.burton@imgtec.com> - 2016-08-30 18:00 +0200
        Re: [PATCH 24/26] dt-bindings: Document img,boston-clock binding Rob Herring <robh@kernel.org> - 2016-09-02 15:00 +0200
          Re: [PATCH 24/26] dt-bindings: Document img,boston-clock binding Paul Burton <paul.burton@imgtec.com> - 2016-09-02 15:40 +0200

#1470835 — [PATCH 24/26] dt-bindings: Document img,boston-clock binding

Add device tree binding documentation for the clocks provided by the
MIPS Boston development board from Imagination Technologies, and a
header file describing the available clocks for use by device trees &
driver.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
---

 .../devicetree/bindings/clock/img,boston-clock.txt | 27 ++++++++++++++++++++++
 include/dt-bindings/clock/boston-clock.h           | 13 +++++++++++
 2 files changed, 40 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/img,boston-clock.txt
 create mode 100644 include/dt-bindings/clock/boston-clock.h

diff --git a/Documentation/devicetree/bindings/clock/img,boston-clock.txt b/Documentation/devicetree/bindings/clock/img,boston-clock.txt
new file mode 100644
index 0000000..c01ea60
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/img,boston-clock.txt
@@ -0,0 +1,27 @@
+Binding for Imagination Technologies MIPS Boston clock sources.
+
+This binding uses the common clock binding[1].
+
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+Required properties:
+- compatible : Should be "img,boston-clock".
+- #clock-cells : Should be set to 1.
+  Values available for clock consumers can be found in the header file:
+    <dt-bindings/clock/boston-clock.h>
+- regmap : Phandle to the Boston platform register system controller.
+  This should contain a phandle to the system controller node covering the
+  platform registers provided by the Boston board.
+
+Example:
+
+	clk_boston: clock {
+		compatible = "img,boston-clock";
+		#clock-cells = <1>;
+		regmap = <&plat_regs>;
+	};
+
+	uart0: uart@17ffe000 {
+		/* ... */
+		clocks = <&clk_boston BOSTON_CLK_SYS>;
+	};
diff --git a/include/dt-bindings/clock/boston-clock.h b/include/dt-bindings/clock/boston-clock.h
new file mode 100644
index 0000000..25f9cd2
--- /dev/null
+++ b/include/dt-bindings/clock/boston-clock.h
@@ -0,0 +1,13 @@
+/*
+ * Copyright (C) 2016 Imagination Technologies
+ *
+ * SPDX-License-Identifier:	GPL-2.0
+ */
+
+#ifndef __DT_BINDINGS_CLOCK_BOSTON_CLOCK_H__
+#define __DT_BINDINGS_CLOCK_BOSTON_CLOCK_H__
+
+#define BOSTON_CLK_SYS 0
+#define BOSTON_CLK_CPU 1
+
+#endif /* __DT_BINDINGS_CLOCK_BOSTON_CLOCK_H__ */
-- 
2.9.3

[toc] | [next] | [standalone]

#1470910

On 08/26, Paul Burton wrote:
> diff --git a/Documentation/devicetree/bindings/clock/img,boston-clock.txt b/Documentation/devicetree/bindings/clock/img,boston-clock.txt
> new file mode 100644
> index 0000000..c01ea60
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/img,boston-clock.txt
> @@ -0,0 +1,27 @@
> +Binding for Imagination Technologies MIPS Boston clock sources.
> +
> +This binding uses the common clock binding[1].
> +
> +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
> +
> +Required properties:
> +- compatible : Should be "img,boston-clock".
> +- #clock-cells : Should be set to 1.
> +  Values available for clock consumers can be found in the header file:
> +    <dt-bindings/clock/boston-clock.h>
> +- regmap : Phandle to the Boston platform register system controller.
> +  This should contain a phandle to the system controller node covering the
> +  platform registers provided by the Boston board.
> +
> +Example:
> +
> +	clk_boston: clock {
> +		compatible = "img,boston-clock";
> +		#clock-cells = <1>;
> +		regmap = <&plat_regs>;

Isn't syscon more standard than regmap as the property name? Is
there a binding for the plat_regs device? Is there any reason the
clks can't be populated in that syscon driver?

> +	};
> +
> +	uart0: uart@17ffe000 {
> +		/* ... */
> +		clocks = <&clk_boston BOSTON_CLK_SYS>;
> +	};

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

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

#1472589

On 26/08/16 18:44, Stephen Boyd wrote:
> On 08/26, Paul Burton wrote:
>> diff --git a/Documentation/devicetree/bindings/clock/img,boston-clock.txt b/Documentation/devicetree/bindings/clock/img,boston-clock.txt
>> new file mode 100644
>> index 0000000..c01ea60
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/img,boston-clock.txt
>> @@ -0,0 +1,27 @@
>> +Binding for Imagination Technologies MIPS Boston clock sources.
>> +
>> +This binding uses the common clock binding[1].
>> +
>> +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
>> +
>> +Required properties:
>> +- compatible : Should be "img,boston-clock".
>> +- #clock-cells : Should be set to 1.
>> +  Values available for clock consumers can be found in the header file:
>> +    <dt-bindings/clock/boston-clock.h>
>> +- regmap : Phandle to the Boston platform register system controller.
>> +  This should contain a phandle to the system controller node covering the
>> +  platform registers provided by the Boston board.
>> +
>> +Example:
>> +
>> +	clk_boston: clock {
>> +		compatible = "img,boston-clock";
>> +		#clock-cells = <1>;
>> +		regmap = <&plat_regs>;
> 
> Isn't syscon more standard than regmap as the property name? Is
> there a binding for the plat_regs device? Is there any reason the
> clks can't be populated in that syscon driver?

Hi Stephen,

The plat_regs device doesn't have a custom driver, it simply makes use
of the generic "syscon" driver which can provide a regmap.

It would be possible to register the clocks from a register for the
plat_regs device, but I don't think it would make much sense. The
platform registers in question are essentially just a convenient place
where various bits of information about the system are exposed,
including the clock frequencies but also other bits & pieces like
connectivity of PCIe controllers or I/O coherence units, the RTL
revision of the CPU or the wrapper RTL that runs on this FPGA-based
board, a register that allows for resetting the board, etc. It's not a
single piece of hardware, more a dumping ground for miscellanea. So in
my opinion using the syscon approach works best here, and drivers for
well defined pieces of hardware or functionality can reference that
syscon to retrieve the regmap.

As for whether "syscon" is a more standard property name than "regmap",
both seem to be used based on a grep of
Documentation/devicetree/bindings/. I believe I picked up use of
"regmap" from the generic syscon-poweroff & syscon-reboot drivers, which
both use "regmap" as a property name.

Thanks,
    Paul

> 
>> +	};
>> +
>> +	uart0: uart@17ffe000 {
>> +		/* ... */
>> +		clocks = <&clk_boston BOSTON_CLK_SYS>;
>> +	};
> 

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

#1475132

On Tue, Aug 30, 2016 at 04:53:01PM +0100, Paul Burton wrote:
> On 26/08/16 18:44, Stephen Boyd wrote:
> > On 08/26, Paul Burton wrote:
> >> diff --git a/Documentation/devicetree/bindings/clock/img,boston-clock.txt b/Documentation/devicetree/bindings/clock/img,boston-clock.txt
> >> new file mode 100644
> >> index 0000000..c01ea60
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/clock/img,boston-clock.txt
> >> @@ -0,0 +1,27 @@
> >> +Binding for Imagination Technologies MIPS Boston clock sources.
> >> +
> >> +This binding uses the common clock binding[1].
> >> +
> >> +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
> >> +
> >> +Required properties:
> >> +- compatible : Should be "img,boston-clock".
> >> +- #clock-cells : Should be set to 1.
> >> +  Values available for clock consumers can be found in the header file:
> >> +    <dt-bindings/clock/boston-clock.h>
> >> +- regmap : Phandle to the Boston platform register system controller.
> >> +  This should contain a phandle to the system controller node covering the
> >> +  platform registers provided by the Boston board.
> >> +
> >> +Example:
> >> +
> >> +	clk_boston: clock {
> >> +		compatible = "img,boston-clock";
> >> +		#clock-cells = <1>;
> >> +		regmap = <&plat_regs>;
> > 
> > Isn't syscon more standard than regmap as the property name? Is
> > there a binding for the plat_regs device? Is there any reason the
> > clks can't be populated in that syscon driver?
> 
> Hi Stephen,
> 
> The plat_regs device doesn't have a custom driver, it simply makes use
> of the generic "syscon" driver which can provide a regmap.
> 
> It would be possible to register the clocks from a register for the
> plat_regs device, but I don't think it would make much sense. The
> platform registers in question are essentially just a convenient place
> where various bits of information about the system are exposed,
> including the clock frequencies but also other bits & pieces like
> connectivity of PCIe controllers or I/O coherence units, the RTL
> revision of the CPU or the wrapper RTL that runs on this FPGA-based
> board, a register that allows for resetting the board, etc. It's not a
> single piece of hardware, more a dumping ground for miscellanea. So in
> my opinion using the syscon approach works best here, and drivers for
> well defined pieces of hardware or functionality can reference that
> syscon to retrieve the regmap.

That is all quite common for any SoC. Whether it's 2 nodes or 2 drivers 
are independent questions. You can easily have 1 node and 2 drivers. The 
decision factor is really how many registers we're dealing with. We 
don't want to end up with a node per register or register field. That's 
too fine grained.

> As for whether "syscon" is a more standard property name than "regmap",
> both seem to be used based on a grep of
> Documentation/devicetree/bindings/. I believe I picked up use of
> "regmap" from the generic syscon-poweroff & syscon-reboot drivers, which
> both use "regmap" as a property name.

syscon is much more common.

Avoid the phandle altogether and make this a child node.

Rob

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

#1475154

On 02/09/16 13:54, Rob Herring wrote:
> On Tue, Aug 30, 2016 at 04:53:01PM +0100, Paul Burton wrote:
>> On 26/08/16 18:44, Stephen Boyd wrote:
>>> On 08/26, Paul Burton wrote:
>>>> diff --git a/Documentation/devicetree/bindings/clock/img,boston-clock.txt b/Documentation/devicetree/bindings/clock/img,boston-clock.txt
>>>> new file mode 100644
>>>> index 0000000..c01ea60
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/clock/img,boston-clock.txt
>>>> @@ -0,0 +1,27 @@
>>>> +Binding for Imagination Technologies MIPS Boston clock sources.
>>>> +
>>>> +This binding uses the common clock binding[1].
>>>> +
>>>> +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
>>>> +
>>>> +Required properties:
>>>> +- compatible : Should be "img,boston-clock".
>>>> +- #clock-cells : Should be set to 1.
>>>> +  Values available for clock consumers can be found in the header file:
>>>> +    <dt-bindings/clock/boston-clock.h>
>>>> +- regmap : Phandle to the Boston platform register system controller.
>>>> +  This should contain a phandle to the system controller node covering the
>>>> +  platform registers provided by the Boston board.
>>>> +
>>>> +Example:
>>>> +
>>>> +	clk_boston: clock {
>>>> +		compatible = "img,boston-clock";
>>>> +		#clock-cells = <1>;
>>>> +		regmap = <&plat_regs>;
>>>
>>> Isn't syscon more standard than regmap as the property name? Is
>>> there a binding for the plat_regs device? Is there any reason the
>>> clks can't be populated in that syscon driver?
>>
>> Hi Stephen,
>>
>> The plat_regs device doesn't have a custom driver, it simply makes use
>> of the generic "syscon" driver which can provide a regmap.
>>
>> It would be possible to register the clocks from a register for the
>> plat_regs device, but I don't think it would make much sense. The
>> platform registers in question are essentially just a convenient place
>> where various bits of information about the system are exposed,
>> including the clock frequencies but also other bits & pieces like
>> connectivity of PCIe controllers or I/O coherence units, the RTL
>> revision of the CPU or the wrapper RTL that runs on this FPGA-based
>> board, a register that allows for resetting the board, etc. It's not a
>> single piece of hardware, more a dumping ground for miscellanea. So in
>> my opinion using the syscon approach works best here, and drivers for
>> well defined pieces of hardware or functionality can reference that
>> syscon to retrieve the regmap.
> 
> That is all quite common for any SoC. Whether it's 2 nodes or 2 drivers 
> are independent questions. You can easily have 1 node and 2 drivers. The 
> decision factor is really how many registers we're dealing with. We 
> don't want to end up with a node per register or register field. That's 
> too fine grained.

Absolutely, I don't think we disagree there.

>> As for whether "syscon" is a more standard property name than "regmap",
>> both seem to be used based on a grep of
>> Documentation/devicetree/bindings/. I believe I picked up use of
>> "regmap" from the generic syscon-poweroff & syscon-reboot drivers, which
>> both use "regmap" as a property name.
> 
> syscon is much more common.
> 
> Avoid the phandle altogether and make this a child node.

I could do that, but it would feel rather odd to describe the clock
hardware as a child of a bunch of miscellaneous registers that happen to
expose some information about those clocks. Is that really what you
prefer? I think as-is the DT is a better description of the hardware.

Thanks,
    Paul

> 
> Rob
> 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web