diff mbox

[1/3] Documentation: dt: Add TI SCI clock driver

Message ID 1477053961-27128-2-git-send-email-t-kristo@ti.com
State Changes Requested, archived
Headers show

Commit Message

Tero Kristo Oct. 21, 2016, 12:45 p.m. UTC
Add a clock implementation, TI SCI clock, that will hook to the common
clock framework, and allow each clock to be controlled via TI SCI
protocol.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
 .../devicetree/bindings/clock/ti,sci-clk.txt       | 37 ++++++++++++++++++++++
 MAINTAINERS                                        |  1 +
 2 files changed, 38 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/ti,sci-clk.txt

Comments

Rob Herring Oct. 30, 2016, 8:41 p.m. UTC | #1
On Fri, Oct 21, 2016 at 03:45:59PM +0300, Tero Kristo wrote:
> Add a clock implementation, TI SCI clock, that will hook to the common
> clock framework, and allow each clock to be controlled via TI SCI
> protocol.
> 
> Signed-off-by: Tero Kristo <t-kristo@ti.com>
> ---
>  .../devicetree/bindings/clock/ti,sci-clk.txt       | 37 ++++++++++++++++++++++
>  MAINTAINERS                                        |  1 +
>  2 files changed, 38 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/ti,sci-clk.txt
> 
> diff --git a/Documentation/devicetree/bindings/clock/ti,sci-clk.txt b/Documentation/devicetree/bindings/clock/ti,sci-clk.txt
> new file mode 100644
> index 0000000..bfc3ca4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/ti,sci-clk.txt
> @@ -0,0 +1,37 @@
> +Texas Instruments TI-SCI Clocks
> +===============================
> +
> +All clocks on Texas Instruments' SoCs that contain a System Controller,
> +are only controlled by this entity. Communication between a host processor
> +running an OS and the System Controller happens through a protocol known
> +as TI-SCI[1]. This clock implementation plugs into the common clock
> +framework and makes use of the TI-SCI protocol on clock API requests.
> +
> +[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
> +
> +Required properties:
> +-------------------
> +- compatible: Must be "ti,k2g-sci-clk"
> +- #clock-cells: Shall be 2.
> +  In clock consumers, this cell represents the device ID and clock ID
> +  exposed by the PM firmware. The assignments can be found in the header
> +  files <dt-bindings/genpd/<soc>.h> (which covers the device IDs) and
> +  <dt-bindings/clock/<soc>.h> (which covers the clock IDs), where <soc>
> +  is the SoC involved, for example 'k2g'.
> +
> +Examples:
> +--------
> +
> +pmmc: pmmc {
> +	compatible = "ti,k2g-sci";
> +
> +	k2g_clks: k2g_clks {

Use "clocks" for node name instead.
 
> +		compatible = "ti,k2g-sci-clk";

I'm starting to think all these child nodes for SCI are pointless. Is 
there any reason why the parent node can't be the clock provider (along 
with all the other providers it acks as)?

> +		#clock-cells = <2>;
> +	};
> +};
> +
> +uart0: serial@2530c00 {
> +	compatible = "ns16550a";
> +	clocks = <&k2g_clks K2G_DEV_UART0 0>;
> +};
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tero Kristo Oct. 31, 2016, 12:50 p.m. UTC | #2
On 30/10/16 22:41, Rob Herring wrote:
> On Fri, Oct 21, 2016 at 03:45:59PM +0300, Tero Kristo wrote:
>> Add a clock implementation, TI SCI clock, that will hook to the common
>> clock framework, and allow each clock to be controlled via TI SCI
>> protocol.
>>
>> Signed-off-by: Tero Kristo <t-kristo@ti.com>
>> ---
>>  .../devicetree/bindings/clock/ti,sci-clk.txt       | 37 ++++++++++++++++++++++
>>  MAINTAINERS                                        |  1 +
>>  2 files changed, 38 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/clock/ti,sci-clk.txt
>>
>> diff --git a/Documentation/devicetree/bindings/clock/ti,sci-clk.txt b/Documentation/devicetree/bindings/clock/ti,sci-clk.txt
>> new file mode 100644
>> index 0000000..bfc3ca4
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/ti,sci-clk.txt
>> @@ -0,0 +1,37 @@
>> +Texas Instruments TI-SCI Clocks
>> +===============================
>> +
>> +All clocks on Texas Instruments' SoCs that contain a System Controller,
>> +are only controlled by this entity. Communication between a host processor
>> +running an OS and the System Controller happens through a protocol known
>> +as TI-SCI[1]. This clock implementation plugs into the common clock
>> +framework and makes use of the TI-SCI protocol on clock API requests.
>> +
>> +[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
>> +
>> +Required properties:
>> +-------------------
>> +- compatible: Must be "ti,k2g-sci-clk"
>> +- #clock-cells: Shall be 2.
>> +  In clock consumers, this cell represents the device ID and clock ID
>> +  exposed by the PM firmware. The assignments can be found in the header
>> +  files <dt-bindings/genpd/<soc>.h> (which covers the device IDs) and
>> +  <dt-bindings/clock/<soc>.h> (which covers the clock IDs), where <soc>
>> +  is the SoC involved, for example 'k2g'.
>> +
>> +Examples:
>> +--------
>> +
>> +pmmc: pmmc {
>> +	compatible = "ti,k2g-sci";
>> +
>> +	k2g_clks: k2g_clks {
>
> Use "clocks" for node name instead.
>
>> +		compatible = "ti,k2g-sci-clk";
>
> I'm starting to think all these child nodes for SCI are pointless. Is
> there any reason why the parent node can't be the clock provider (along
> with all the other providers it acks as)?

I believe the only reason to keep them separate is to have kernel side 
of things modular. If we have separate nodes, the drivers can be probed 
separately.

If not, we need to build one huge blob with all the features in it, so 
the main driver can probe everything in one go, with annoying 
back-and-forth callbacks in place (assuming we still want to keep stuff 
somehow modular.)

-Tero

>
>> +		#clock-cells = <2>;
>> +	};
>> +};
>> +
>> +uart0: serial@2530c00 {
>> +	compatible = "ns16550a";
>> +	clocks = <&k2g_clks K2G_DEV_UART0 0>;
>> +};

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Nishanth Menon Oct. 31, 2016, 8:34 p.m. UTC | #3
On 10/31/2016 07:50 AM, Tero Kristo wrote:
[...]

>>> +pmmc: pmmc {
>>> +	compatible = "ti,k2g-sci";
>>> +
>>> +	k2g_clks: k2g_clks {
>>
>> Use "clocks" for node name instead.
>>
>>> +		compatible = "ti,k2g-sci-clk";
>>
>> I'm starting to think all these child nodes for SCI are pointless. Is
>> there any reason why the parent node can't be the clock provider (along
>> with all the other providers it acks as)?
>
> I believe the only reason to keep them separate is to have kernel side
> of things modular. If we have separate nodes, the drivers can be probed
> separately.
>
> If not, we need to build one huge blob with all the features in it, so
> the main driver can probe everything in one go, with annoying
> back-and-forth callbacks in place (assuming we still want to keep stuff
> somehow modular.)

Documentation/devicetree/bindings/arm/arm,scpi.txt follows the same 
solution as well, right? There is indeed additional nodes coming in - 
such as reset, pd etc.. I cant see why it is different for sci clk.. 
not to mention the driver mess it results in.
Rob Herring Nov. 18, 2016, 5:20 p.m. UTC | #4
On Mon, Oct 31, 2016 at 7:50 AM, Tero Kristo <t-kristo@ti.com> wrote:
> On 30/10/16 22:41, Rob Herring wrote:
>>
>> On Fri, Oct 21, 2016 at 03:45:59PM +0300, Tero Kristo wrote:
>>>
>>> Add a clock implementation, TI SCI clock, that will hook to the common
>>> clock framework, and allow each clock to be controlled via TI SCI
>>> protocol.
>>>
>>> Signed-off-by: Tero Kristo <t-kristo@ti.com>
>>> ---
>>>  .../devicetree/bindings/clock/ti,sci-clk.txt       | 37
>>> ++++++++++++++++++++++
>>>  MAINTAINERS                                        |  1 +
>>>  2 files changed, 38 insertions(+)
>>>  create mode 100644
>>> Documentation/devicetree/bindings/clock/ti,sci-clk.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/clock/ti,sci-clk.txt
>>> b/Documentation/devicetree/bindings/clock/ti,sci-clk.txt
>>> new file mode 100644
>>> index 0000000..bfc3ca4
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/clock/ti,sci-clk.txt
>>> @@ -0,0 +1,37 @@
>>> +Texas Instruments TI-SCI Clocks
>>> +===============================
>>> +
>>> +All clocks on Texas Instruments' SoCs that contain a System Controller,
>>> +are only controlled by this entity. Communication between a host
>>> processor
>>> +running an OS and the System Controller happens through a protocol known
>>> +as TI-SCI[1]. This clock implementation plugs into the common clock
>>> +framework and makes use of the TI-SCI protocol on clock API requests.
>>> +
>>> +[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
>>> +
>>> +Required properties:
>>> +-------------------
>>> +- compatible: Must be "ti,k2g-sci-clk"
>>> +- #clock-cells: Shall be 2.
>>> +  In clock consumers, this cell represents the device ID and clock ID
>>> +  exposed by the PM firmware. The assignments can be found in the header
>>> +  files <dt-bindings/genpd/<soc>.h> (which covers the device IDs) and
>>> +  <dt-bindings/clock/<soc>.h> (which covers the clock IDs), where <soc>
>>> +  is the SoC involved, for example 'k2g'.
>>> +
>>> +Examples:
>>> +--------
>>> +
>>> +pmmc: pmmc {
>>> +       compatible = "ti,k2g-sci";
>>> +
>>> +       k2g_clks: k2g_clks {
>>
>>
>> Use "clocks" for node name instead.
>>
>>> +               compatible = "ti,k2g-sci-clk";
>>
>>
>> I'm starting to think all these child nodes for SCI are pointless. Is
>> there any reason why the parent node can't be the clock provider (along
>> with all the other providers it acks as)?
>
>
> I believe the only reason to keep them separate is to have kernel side of
> things modular. If we have separate nodes, the drivers can be probed
> separately.
>
> If not, we need to build one huge blob with all the features in it, so the
> main driver can probe everything in one go, with annoying back-and-forth
> callbacks in place (assuming we still want to keep stuff somehow modular.)

Since when is DT the only way to create a device? The main driver can
create devices for all the sub-functions like clocks. This is the same
as MFDs which have been done both ways.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tero Kristo Nov. 21, 2016, 8:14 a.m. UTC | #5
On 18/11/16 19:20, Rob Herring wrote:
> On Mon, Oct 31, 2016 at 7:50 AM, Tero Kristo <t-kristo@ti.com> wrote:
>> On 30/10/16 22:41, Rob Herring wrote:
>>>
>>> On Fri, Oct 21, 2016 at 03:45:59PM +0300, Tero Kristo wrote:
>>>>
>>>> Add a clock implementation, TI SCI clock, that will hook to the common
>>>> clock framework, and allow each clock to be controlled via TI SCI
>>>> protocol.
>>>>
>>>> Signed-off-by: Tero Kristo <t-kristo@ti.com>
>>>> ---
>>>>  .../devicetree/bindings/clock/ti,sci-clk.txt       | 37
>>>> ++++++++++++++++++++++
>>>>  MAINTAINERS                                        |  1 +
>>>>  2 files changed, 38 insertions(+)
>>>>  create mode 100644
>>>> Documentation/devicetree/bindings/clock/ti,sci-clk.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/clock/ti,sci-clk.txt
>>>> b/Documentation/devicetree/bindings/clock/ti,sci-clk.txt
>>>> new file mode 100644
>>>> index 0000000..bfc3ca4
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/clock/ti,sci-clk.txt
>>>> @@ -0,0 +1,37 @@
>>>> +Texas Instruments TI-SCI Clocks
>>>> +===============================
>>>> +
>>>> +All clocks on Texas Instruments' SoCs that contain a System Controller,
>>>> +are only controlled by this entity. Communication between a host
>>>> processor
>>>> +running an OS and the System Controller happens through a protocol known
>>>> +as TI-SCI[1]. This clock implementation plugs into the common clock
>>>> +framework and makes use of the TI-SCI protocol on clock API requests.
>>>> +
>>>> +[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
>>>> +
>>>> +Required properties:
>>>> +-------------------
>>>> +- compatible: Must be "ti,k2g-sci-clk"
>>>> +- #clock-cells: Shall be 2.
>>>> +  In clock consumers, this cell represents the device ID and clock ID
>>>> +  exposed by the PM firmware. The assignments can be found in the header
>>>> +  files <dt-bindings/genpd/<soc>.h> (which covers the device IDs) and
>>>> +  <dt-bindings/clock/<soc>.h> (which covers the clock IDs), where <soc>
>>>> +  is the SoC involved, for example 'k2g'.
>>>> +
>>>> +Examples:
>>>> +--------
>>>> +
>>>> +pmmc: pmmc {
>>>> +       compatible = "ti,k2g-sci";
>>>> +
>>>> +       k2g_clks: k2g_clks {
>>>
>>>
>>> Use "clocks" for node name instead.
>>>
>>>> +               compatible = "ti,k2g-sci-clk";
>>>
>>>
>>> I'm starting to think all these child nodes for SCI are pointless. Is
>>> there any reason why the parent node can't be the clock provider (along
>>> with all the other providers it acks as)?
>>
>>
>> I believe the only reason to keep them separate is to have kernel side of
>> things modular. If we have separate nodes, the drivers can be probed
>> separately.
>>
>> If not, we need to build one huge blob with all the features in it, so the
>> main driver can probe everything in one go, with annoying back-and-forth
>> callbacks in place (assuming we still want to keep stuff somehow modular.)
>
> Since when is DT the only way to create a device? The main driver can
> create devices for all the sub-functions like clocks. This is the same
> as MFDs which have been done both ways.

Yes obviously this can be done, my main point was that it will require 
building some sort of infra within the driver to handle this. With 
separate nodes, none of this is going to be needed. Also, we will lose 
any kind of configurability via DT if we don't have separate nodes; now 
we can select the available clocks / genpds via the compatible string of 
the clocks/genpd nodes themselves (this isn't clearly evident as of now 
as we only support a grand total of one device, which is k2g-evm.) 
Otherwise we need to probe against the main node and add a separate 
compatible string for every device, and carry this information to the 
sibling devices also somehow. It is just so much simpler if we can just 
keep separate nodes for them.

Also, plenty of things are doing this kind of stuff already in 
DT/kernel, having a parent node in place and sub-functions added 
separately for ease of use, with apparently no visible point for having 
the nodes within the DT.

-Tero
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tero Kristo Dec. 2, 2016, 8:19 a.m. UTC | #6
On 21/11/16 10:14, Tero Kristo wrote:
> On 18/11/16 19:20, Rob Herring wrote:
>> On Mon, Oct 31, 2016 at 7:50 AM, Tero Kristo <t-kristo@ti.com> wrote:
>>> On 30/10/16 22:41, Rob Herring wrote:
>>>>
>>>> On Fri, Oct 21, 2016 at 03:45:59PM +0300, Tero Kristo wrote:
>>>>>
>>>>> Add a clock implementation, TI SCI clock, that will hook to the common
>>>>> clock framework, and allow each clock to be controlled via TI SCI
>>>>> protocol.
>>>>>
>>>>> Signed-off-by: Tero Kristo <t-kristo@ti.com>
>>>>> ---
>>>>>  .../devicetree/bindings/clock/ti,sci-clk.txt       | 37
>>>>> ++++++++++++++++++++++
>>>>>  MAINTAINERS                                        |  1 +
>>>>>  2 files changed, 38 insertions(+)
>>>>>  create mode 100644
>>>>> Documentation/devicetree/bindings/clock/ti,sci-clk.txt
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/clock/ti,sci-clk.txt
>>>>> b/Documentation/devicetree/bindings/clock/ti,sci-clk.txt
>>>>> new file mode 100644
>>>>> index 0000000..bfc3ca4
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/clock/ti,sci-clk.txt
>>>>> @@ -0,0 +1,37 @@
>>>>> +Texas Instruments TI-SCI Clocks
>>>>> +===============================
>>>>> +
>>>>> +All clocks on Texas Instruments' SoCs that contain a System
>>>>> Controller,
>>>>> +are only controlled by this entity. Communication between a host
>>>>> processor
>>>>> +running an OS and the System Controller happens through a protocol
>>>>> known
>>>>> +as TI-SCI[1]. This clock implementation plugs into the common clock
>>>>> +framework and makes use of the TI-SCI protocol on clock API requests.
>>>>> +
>>>>> +[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
>>>>> +
>>>>> +Required properties:
>>>>> +-------------------
>>>>> +- compatible: Must be "ti,k2g-sci-clk"
>>>>> +- #clock-cells: Shall be 2.
>>>>> +  In clock consumers, this cell represents the device ID and clock ID
>>>>> +  exposed by the PM firmware. The assignments can be found in the
>>>>> header
>>>>> +  files <dt-bindings/genpd/<soc>.h> (which covers the device IDs) and
>>>>> +  <dt-bindings/clock/<soc>.h> (which covers the clock IDs), where
>>>>> <soc>
>>>>> +  is the SoC involved, for example 'k2g'.
>>>>> +
>>>>> +Examples:
>>>>> +--------
>>>>> +
>>>>> +pmmc: pmmc {
>>>>> +       compatible = "ti,k2g-sci";
>>>>> +
>>>>> +       k2g_clks: k2g_clks {
>>>>
>>>>
>>>> Use "clocks" for node name instead.
>>>>
>>>>> +               compatible = "ti,k2g-sci-clk";
>>>>
>>>>
>>>> I'm starting to think all these child nodes for SCI are pointless. Is
>>>> there any reason why the parent node can't be the clock provider (along
>>>> with all the other providers it acks as)?
>>>
>>>
>>> I believe the only reason to keep them separate is to have kernel
>>> side of
>>> things modular. If we have separate nodes, the drivers can be probed
>>> separately.
>>>
>>> If not, we need to build one huge blob with all the features in it,
>>> so the
>>> main driver can probe everything in one go, with annoying back-and-forth
>>> callbacks in place (assuming we still want to keep stuff somehow
>>> modular.)
>>
>> Since when is DT the only way to create a device? The main driver can
>> create devices for all the sub-functions like clocks. This is the same
>> as MFDs which have been done both ways.
>
> Yes obviously this can be done, my main point was that it will require
> building some sort of infra within the driver to handle this. With
> separate nodes, none of this is going to be needed. Also, we will lose
> any kind of configurability via DT if we don't have separate nodes; now
> we can select the available clocks / genpds via the compatible string of
> the clocks/genpd nodes themselves (this isn't clearly evident as of now
> as we only support a grand total of one device, which is k2g-evm.)
> Otherwise we need to probe against the main node and add a separate
> compatible string for every device, and carry this information to the
> sibling devices also somehow. It is just so much simpler if we can just
> keep separate nodes for them.
>
> Also, plenty of things are doing this kind of stuff already in
> DT/kernel, having a parent node in place and sub-functions added
> separately for ease of use, with apparently no visible point for having
> the nodes within the DT.

Rob, any response on this one? I see you have acked the reset part of 
the bindings which is doing pretty much the same thing as the clock part 
is doing here, namely adding child node under the main SCI node. Is it 
okay to do this same for other parts of the TI SCI?

-Tero
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rob Herring Dec. 2, 2016, 6:45 p.m. UTC | #7
On Fri, Dec 2, 2016 at 2:19 AM, Tero Kristo <t-kristo@ti.com> wrote:
> On 21/11/16 10:14, Tero Kristo wrote:
>>
>> On 18/11/16 19:20, Rob Herring wrote:
>>>
>>> On Mon, Oct 31, 2016 at 7:50 AM, Tero Kristo <t-kristo@ti.com> wrote:
>>>>
>>>> On 30/10/16 22:41, Rob Herring wrote:
>>>>>
>>>>>
>>>>> On Fri, Oct 21, 2016 at 03:45:59PM +0300, Tero Kristo wrote:
>>>>>>
>>>>>>
>>>>>> Add a clock implementation, TI SCI clock, that will hook to the common
>>>>>> clock framework, and allow each clock to be controlled via TI SCI
>>>>>> protocol.
>>>>>>
>>>>>> Signed-off-by: Tero Kristo <t-kristo@ti.com>
>>>>>> ---
>>>>>>  .../devicetree/bindings/clock/ti,sci-clk.txt       | 37
>>>>>> ++++++++++++++++++++++
>>>>>>  MAINTAINERS                                        |  1 +
>>>>>>  2 files changed, 38 insertions(+)
>>>>>>  create mode 100644
>>>>>> Documentation/devicetree/bindings/clock/ti,sci-clk.txt
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/clock/ti,sci-clk.txt
>>>>>> b/Documentation/devicetree/bindings/clock/ti,sci-clk.txt
>>>>>> new file mode 100644
>>>>>> index 0000000..bfc3ca4
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/clock/ti,sci-clk.txt
>>>>>> @@ -0,0 +1,37 @@
>>>>>> +Texas Instruments TI-SCI Clocks
>>>>>> +===============================
>>>>>> +
>>>>>> +All clocks on Texas Instruments' SoCs that contain a System
>>>>>> Controller,
>>>>>> +are only controlled by this entity. Communication between a host
>>>>>> processor
>>>>>> +running an OS and the System Controller happens through a protocol
>>>>>> known
>>>>>> +as TI-SCI[1]. This clock implementation plugs into the common clock
>>>>>> +framework and makes use of the TI-SCI protocol on clock API requests.
>>>>>> +
>>>>>> +[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
>>>>>> +
>>>>>> +Required properties:
>>>>>> +-------------------
>>>>>> +- compatible: Must be "ti,k2g-sci-clk"
>>>>>> +- #clock-cells: Shall be 2.
>>>>>> +  In clock consumers, this cell represents the device ID and clock ID
>>>>>> +  exposed by the PM firmware. The assignments can be found in the
>>>>>> header
>>>>>> +  files <dt-bindings/genpd/<soc>.h> (which covers the device IDs) and
>>>>>> +  <dt-bindings/clock/<soc>.h> (which covers the clock IDs), where
>>>>>> <soc>
>>>>>> +  is the SoC involved, for example 'k2g'.
>>>>>> +
>>>>>> +Examples:
>>>>>> +--------
>>>>>> +
>>>>>> +pmmc: pmmc {
>>>>>> +       compatible = "ti,k2g-sci";
>>>>>> +
>>>>>> +       k2g_clks: k2g_clks {
>>>>>
>>>>>
>>>>>
>>>>> Use "clocks" for node name instead.
>>>>>
>>>>>> +               compatible = "ti,k2g-sci-clk";
>>>>>
>>>>>
>>>>>
>>>>> I'm starting to think all these child nodes for SCI are pointless. Is
>>>>> there any reason why the parent node can't be the clock provider (along
>>>>> with all the other providers it acks as)?
>>>>
>>>>
>>>>
>>>> I believe the only reason to keep them separate is to have kernel
>>>> side of
>>>> things modular. If we have separate nodes, the drivers can be probed
>>>> separately.
>>>>
>>>> If not, we need to build one huge blob with all the features in it,
>>>> so the
>>>> main driver can probe everything in one go, with annoying back-and-forth
>>>> callbacks in place (assuming we still want to keep stuff somehow
>>>> modular.)
>>>
>>>
>>> Since when is DT the only way to create a device? The main driver can
>>> create devices for all the sub-functions like clocks. This is the same
>>> as MFDs which have been done both ways.
>>
>>
>> Yes obviously this can be done, my main point was that it will require
>> building some sort of infra within the driver to handle this. With
>> separate nodes, none of this is going to be needed. Also, we will lose
>> any kind of configurability via DT if we don't have separate nodes; now
>> we can select the available clocks / genpds via the compatible string of
>> the clocks/genpd nodes themselves (this isn't clearly evident as of now
>> as we only support a grand total of one device, which is k2g-evm.)
>> Otherwise we need to probe against the main node and add a separate
>> compatible string for every device, and carry this information to the
>> sibling devices also somehow. It is just so much simpler if we can just
>> keep separate nodes for them.
>>
>> Also, plenty of things are doing this kind of stuff already in
>> DT/kernel, having a parent node in place and sub-functions added
>> separately for ease of use, with apparently no visible point for having
>> the nodes within the DT.
>
>
> Rob, any response on this one? I see you have acked the reset part of the
> bindings which is doing pretty much the same thing as the clock part is
> doing here, namely adding child node under the main SCI node. Is it okay to
> do this same for other parts of the TI SCI?

Yes. It would be silly to allow for one and not others...

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Boyd Dec. 2, 2016, 6:58 p.m. UTC | #8
On 12/02/2016 10:45 AM, Rob Herring wrote:
> On Fri, Dec 2, 2016 at 2:19 AM, Tero Kristo <t-kristo@ti.com> wrote:
>>
>> Rob, any response on this one? I see you have acked the reset part of the
>> bindings which is doing pretty much the same thing as the clock part is
>> doing here, namely adding child node under the main SCI node. Is it okay to
>> do this same for other parts of the TI SCI?
> Yes. It would be silly to allow for one and not others...
>

I'm expecting a respin for the node name (clocks or clock-controller).
I'll also make a review pass on patch 3 today so please don't respin
until after that.
Tero Kristo Dec. 2, 2016, 9:07 p.m. UTC | #9
On 02/12/16 20:58, Stephen Boyd wrote:
> On 12/02/2016 10:45 AM, Rob Herring wrote:
>> On Fri, Dec 2, 2016 at 2:19 AM, Tero Kristo <t-kristo@ti.com> wrote:
>>>
>>> Rob, any response on this one? I see you have acked the reset part of the
>>> bindings which is doing pretty much the same thing as the clock part is
>>> doing here, namely adding child node under the main SCI node. Is it okay to
>>> do this same for other parts of the TI SCI?
>> Yes. It would be silly to allow for one and not others...
>>
>
> I'm expecting a respin for the node name (clocks or clock-controller).
> I'll also make a review pass on patch 3 today so please don't respin
> until after that.

Yeah, I need to fix that and re-send. This series will be most likely 
delayed until 4.11 though seeing its very late in 4.9-rc already (and 
the genpd part dependency is still not ready either) so we are not in a 
rush right now.

-Tero

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

Patch

diff --git a/Documentation/devicetree/bindings/clock/ti,sci-clk.txt b/Documentation/devicetree/bindings/clock/ti,sci-clk.txt
new file mode 100644
index 0000000..bfc3ca4
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/ti,sci-clk.txt
@@ -0,0 +1,37 @@ 
+Texas Instruments TI-SCI Clocks
+===============================
+
+All clocks on Texas Instruments' SoCs that contain a System Controller,
+are only controlled by this entity. Communication between a host processor
+running an OS and the System Controller happens through a protocol known
+as TI-SCI[1]. This clock implementation plugs into the common clock
+framework and makes use of the TI-SCI protocol on clock API requests.
+
+[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
+
+Required properties:
+-------------------
+- compatible: Must be "ti,k2g-sci-clk"
+- #clock-cells: Shall be 2.
+  In clock consumers, this cell represents the device ID and clock ID
+  exposed by the PM firmware. The assignments can be found in the header
+  files <dt-bindings/genpd/<soc>.h> (which covers the device IDs) and
+  <dt-bindings/clock/<soc>.h> (which covers the clock IDs), where <soc>
+  is the SoC involved, for example 'k2g'.
+
+Examples:
+--------
+
+pmmc: pmmc {
+	compatible = "ti,k2g-sci";
+
+	k2g_clks: k2g_clks {
+		compatible = "ti,k2g-sci-clk";
+		#clock-cells = <2>;
+	};
+};
+
+uart0: serial@2530c00 {
+	compatible = "ns16550a";
+	clocks = <&k2g_clks K2G_DEV_UART0 0>;
+};
diff --git a/MAINTAINERS b/MAINTAINERS
index 3eaac5ed..3ee7c7a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11895,6 +11895,7 @@  F:	include/linux/soc/ti/ti_sci_protocol.h
 F:	Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
 F:	include/dt-bindings/genpd/k2g.h
 F:	drivers/soc/ti/ti_sci_pm_domains.c
+F:	Documentation/devicetree/bindings/clock/ti,sci-clk.txt
 
 THANKO'S RAREMONO AM/FM/SW RADIO RECEIVER USB DRIVER
 M:	Hans Verkuil <hverkuil@xs4all.nl>