[GIT PULL] DT clk binding support

[GIT PULL] DT clk binding support

[Rob Herring]

Mike,

Please pull devicetree clock binding support for 3.5. It's a bit late I
know, but this shouldn't affect anyone except for highbank which uses
it. It will be good to get it in so other platforms can start using it.

If you prefer, perhaps Arnd/Olof can pull this directly if you don't
have any other pulls planned for 3.5-rc1.

Rob

The following changes since commit f0948f59dbc8e725a96ba16da666e8f5cdd43ba8:

  clk: add a fixed factor clock (2012-05-08 14:13:25 -0700)

are available in the git repository at:

  git://sources.calxeda.com/kernel/linux.git clk-for-3.5

for you to fetch changes up to 9dca208955ffba16adc99d982fd7432b37f76faa:

  clk: add highbank clock support (2012-05-15 19:14:10 -0500)

----------------------------------------------------------------
Grant Likely (2):
      clk: add DT clock binding support
      clk: add DT fixed-clock binding support

Rob Herring (2):
      dt: add clock binding doc to primecell bindings
      clk: add highbank clock support

 .../devicetree/bindings/arm/primecell.txt          |    6 +
 .../devicetree/bindings/clock/calxeda.txt          |   17 +
 .../devicetree/bindings/clock/clock-bindings.txt   |  117 +++++++
 .../devicetree/bindings/clock/fixed-clock.txt      |   21 ++
 arch/arm/Kconfig                                   |    1 +
 arch/arm/boot/dts/highbank.dts                     |   91 +++++-
 arch/arm/mach-highbank/Makefile                    |    2 +-
 arch/arm/mach-highbank/clock.c                     |   62 ----
 arch/arm/mach-highbank/highbank.c                  |    7 +
 drivers/clk/Makefile                               |    4 +-
 drivers/clk/clk-fixed-rate.c                       |   23 ++
 drivers/clk/clk-highbank.c                         |  345
++++++++++++++++++++
 drivers/clk/clk.c                                  |  140 ++++++++
 drivers/clk/clkdev.c                               |   77 +++++
 include/linux/clk-provider.h                       |   16 +
 include/linux/clk.h                                |   19 ++
 16 files changed, 883 insertions(+), 65 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/calxeda.txt
 create mode 100644
Documentation/devicetree/bindings/clock/clock-bindings.txt
 create mode 100644 Documentation/devicetree/bindings/clock/fixed-clock.txt
 delete mode 100644 arch/arm/mach-highbank/clock.c
 create mode 100644 drivers/clk/clk-highbank.c

[GIT PULL] DT clk binding support

[Rob Herring]

Mike,

Please pull devicetree clock binding support for 3.5. It's a bit late I
know, but this shouldn't affect anyone except for highbank which uses
it. It will be good to get it in so other platforms can start using it.

If you prefer, perhaps Arnd/Olof can pull this directly if you don't
have any other pulls planned for 3.5-rc1.

Rob

The following changes since commit f0948f59dbc8e725a96ba16da666e8f5cdd43ba8:

  clk: add a fixed factor clock (2012-05-08 14:13:25 -0700)

are available in the git repository at:

  git://sources.calxeda.com/kernel/linux.git clk-for-3.5

for you to fetch changes up to 9dca208955ffba16adc99d982fd7432b37f76faa:

  clk: add highbank clock support (2012-05-15 19:14:10 -0500)

----------------------------------------------------------------
Grant Likely (2):
      clk: add DT clock binding support
      clk: add DT fixed-clock binding support

Rob Herring (2):
      dt: add clock binding doc to primecell bindings
      clk: add highbank clock support

 .../devicetree/bindings/arm/primecell.txt          |    6 +
 .../devicetree/bindings/clock/calxeda.txt          |   17 +
 .../devicetree/bindings/clock/clock-bindings.txt   |  117 +++++++
 .../devicetree/bindings/clock/fixed-clock.txt      |   21 ++
 arch/arm/Kconfig                                   |    1 +
 arch/arm/boot/dts/highbank.dts                     |   91 +++++-
 arch/arm/mach-highbank/Makefile                    |    2 +-
 arch/arm/mach-highbank/clock.c                     |   62 ----
 arch/arm/mach-highbank/highbank.c                  |    7 +
 drivers/clk/Makefile                               |    4 +-
 drivers/clk/clk-fixed-rate.c                       |   23 ++
 drivers/clk/clk-highbank.c                         |  345
++++++++++++++++++++
 drivers/clk/clk.c                                  |  140 ++++++++
 drivers/clk/clkdev.c                               |   77 +++++
 include/linux/clk-provider.h                       |   16 +
 include/linux/clk.h                                |   19 ++
 16 files changed, 883 insertions(+), 65 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/calxeda.txt
 create mode 100644
Documentation/devicetree/bindings/clock/clock-bindings.txt
 create mode 100644 Documentation/devicetree/bindings/clock/fixed-clock.txt
 delete mode 100644 arch/arm/mach-highbank/clock.c
 create mode 100644 drivers/clk/clk-highbank.c

Re: [GIT PULL] DT clk binding support

[Shawn Guo]

On Sat, May 19, 2012 at 04:22:58PM -0500, Rob Herring wrote:
> Grant Likely (2):
>       clk: add DT clock binding support

I just checked your branch and found that a couple of comments that
I put on this patch haven't get addressed.  The most notable one would
be the "clocks" property of clock consumers.

+==Clock consumers==
+
+Required properties:
+clocks:                List of phandle and clock specifier pairs, one pair
+               for each clock input to the device.  Note: if the
+               clock provider specifies '0' for #clock-cells, then
+               only the phandle portion of the pair will appear.
+
+Optional properties:
+clock-names:   List of clock input name strings sorted in the same
+               order as the clocks property.  Consumers drivers
+               will use clock-names to match clock input names
+               with clocks specifiers.

As I said, any clock in the clock tree except root clock is not only
a clock provider but also a consumer.  If you define "clocks" as a
required property for clock consumers, you are essentially asking users
to either define the whole clock tree in the device tree or stay away
from device tree completely.

Are you sure this is the right thing to do?  If I remember correctly,
Grant's position is it should be pretty reasonable to have most of
the clock tree defined in clock driver and only define those leaf
clocks which are very likely to become the clock providers for other
peripherals.

Let me put a terrible example here.  Since clock tree is actually SoC
specific, I can reasonably choose to define the entire imx6q clock tree
and all the clk lookups for imx6q peripherals in clk-imx6q driver.
On imx6q-sabrelite board, the audio codec sgtl5000 uses cko (an imx6q
clock available on pad) as the clock source.  That said, I need a board
specific clk lookup here, which should be the best user of clock DT
bindings.  But sadly, with the current bindings, I can not give the
required "clocks" property for sgtl5000 node.

-- 
Regards,
Shawn

[GIT PULL] DT clk binding support

[Shawn Guo]

On Sat, May 19, 2012 at 04:22:58PM -0500, Rob Herring wrote:
> Grant Likely (2):
>       clk: add DT clock binding support

I just checked your branch and found that a couple of comments that
I put on this patch haven't get addressed.  The most notable one would
be the "clocks" property of clock consumers.

+==Clock consumers==
+
+Required properties:
+clocks:                List of phandle and clock specifier pairs, one pair
+               for each clock input to the device.  Note: if the
+               clock provider specifies '0' for #clock-cells, then
+               only the phandle portion of the pair will appear.
+
+Optional properties:
+clock-names:   List of clock input name strings sorted in the same
+               order as the clocks property.  Consumers drivers
+               will use clock-names to match clock input names
+               with clocks specifiers.

As I said, any clock in the clock tree except root clock is not only
a clock provider but also a consumer.  If you define "clocks" as a
required property for clock consumers, you are essentially asking users
to either define the whole clock tree in the device tree or stay away
from device tree completely.

Are you sure this is the right thing to do?  If I remember correctly,
Grant's position is it should be pretty reasonable to have most of
the clock tree defined in clock driver and only define those leaf
clocks which are very likely to become the clock providers for other
peripherals.

Let me put a terrible example here.  Since clock tree is actually SoC
specific, I can reasonably choose to define the entire imx6q clock tree
and all the clk lookups for imx6q peripherals in clk-imx6q driver.
On imx6q-sabrelite board, the audio codec sgtl5000 uses cko (an imx6q
clock available on pad) as the clock source.  That said, I need a board
specific clk lookup here, which should be the best user of clock DT
bindings.  But sadly, with the current bindings, I can not give the
required "clocks" property for sgtl5000 node.

-- 
Regards,
Shawn

Re: [GIT PULL] DT clk binding support

[Rob Herring]

On 05/19/2012 10:06 PM, Shawn Guo wrote:
> On Sat, May 19, 2012 at 04:22:58PM -0500, Rob Herring wrote:
>> Grant Likely (2):
>>       clk: add DT clock binding support
> 
> I just checked your branch and found that a couple of comments that
> I put on this patch haven't get addressed.  The most notable one would
> be the "clocks" property of clock consumers.
> 
> +==Clock consumers==
> +
> +Required properties:
> +clocks:                List of phandle and clock specifier pairs, one pair
> +               for each clock input to the device.  Note: if the
> +               clock provider specifies '0' for #clock-cells, then
> +               only the phandle portion of the pair will appear.
> +
> +Optional properties:
> +clock-names:   List of clock input name strings sorted in the same
> +               order as the clocks property.  Consumers drivers
> +               will use clock-names to match clock input names
> +               with clocks specifiers.
> 
> As I said, any clock in the clock tree except root clock is not only
> a clock provider but also a consumer.  If you define "clocks" as a
> required property for clock consumers, you are essentially asking users
> to either define the whole clock tree in the device tree or stay away
> from device tree completely.

So what are you proposing that a clock consumer have? The very
definition of a clock consumer is that it has a clocks property.

> 
> Are you sure this is the right thing to do?  If I remember correctly,
> Grant's position is it should be pretty reasonable to have most of
> the clock tree defined in clock driver and only define those leaf
> clocks which are very likely to become the clock providers for other
> peripherals.

The minimum is you have to have a provider and consumer. It may be a
single provider that provides all clocks for a chip. If you don't want a
provider, then just define a clock-frequency property.
> 
> Let me put a terrible example here.  Since clock tree is actually SoC
> specific, I can reasonably choose to define the entire imx6q clock tree
> and all the clk lookups for imx6q peripherals in clk-imx6q driver.
> On imx6q-sabrelite board, the audio codec sgtl5000 uses cko (an imx6q
> clock available on pad) as the clock source.  That said, I need a board
> specific clk lookup here, which should be the best user of clock DT
> bindings.  But sadly, with the current bindings, I can not give the
> required "clocks" property for sgtl5000 node.

I don't understand your example. For the sgtl5000 on the sabrelite, you
would provide a phandle and cell entry that is interpreted as the cko
pin. Is your concern the node itself has to change from board to board?
That's normal for all bindings. If you have a module, its interrupts
binding is dependent on the interrupt controller's binding (i.e. a GIC
has 3 cells and VIC has 1 cell) which can vary.

Rob

[GIT PULL] DT clk binding support

[Rob Herring]

On 05/19/2012 10:06 PM, Shawn Guo wrote:
> On Sat, May 19, 2012 at 04:22:58PM -0500, Rob Herring wrote:
>> Grant Likely (2):
>>       clk: add DT clock binding support
> 
> I just checked your branch and found that a couple of comments that
> I put on this patch haven't get addressed.  The most notable one would
> be the "clocks" property of clock consumers.
> 
> +==Clock consumers==
> +
> +Required properties:
> +clocks:                List of phandle and clock specifier pairs, one pair
> +               for each clock input to the device.  Note: if the
> +               clock provider specifies '0' for #clock-cells, then
> +               only the phandle portion of the pair will appear.
> +
> +Optional properties:
> +clock-names:   List of clock input name strings sorted in the same
> +               order as the clocks property.  Consumers drivers
> +               will use clock-names to match clock input names
> +               with clocks specifiers.
> 
> As I said, any clock in the clock tree except root clock is not only
> a clock provider but also a consumer.  If you define "clocks" as a
> required property for clock consumers, you are essentially asking users
> to either define the whole clock tree in the device tree or stay away
> from device tree completely.

So what are you proposing that a clock consumer have? The very
definition of a clock consumer is that it has a clocks property.

> 
> Are you sure this is the right thing to do?  If I remember correctly,
> Grant's position is it should be pretty reasonable to have most of
> the clock tree defined in clock driver and only define those leaf
> clocks which are very likely to become the clock providers for other
> peripherals.

The minimum is you have to have a provider and consumer. It may be a
single provider that provides all clocks for a chip. If you don't want a
provider, then just define a clock-frequency property.
> 
> Let me put a terrible example here.  Since clock tree is actually SoC
> specific, I can reasonably choose to define the entire imx6q clock tree
> and all the clk lookups for imx6q peripherals in clk-imx6q driver.
> On imx6q-sabrelite board, the audio codec sgtl5000 uses cko (an imx6q
> clock available on pad) as the clock source.  That said, I need a board
> specific clk lookup here, which should be the best user of clock DT
> bindings.  But sadly, with the current bindings, I can not give the
> required "clocks" property for sgtl5000 node.

I don't understand your example. For the sgtl5000 on the sabrelite, you
would provide a phandle and cell entry that is interpreted as the cko
pin. Is your concern the node itself has to change from board to board?
That's normal for all bindings. If you have a module, its interrupts
binding is dependent on the interrupt controller's binding (i.e. a GIC
has 3 cells and VIC has 1 cell) which can vary.

Rob

Re: [GIT PULL] DT clk binding support

[Shawn Guo]

On Sun, May 20, 2012 at 09:18:15PM -0500, Rob Herring wrote:
> > As I said, any clock in the clock tree except root clock is not only
> > a clock provider but also a consumer.  If you define "clocks" as a
> > required property for clock consumers, you are essentially asking users
> > to either define the whole clock tree in the device tree or stay away
> > from device tree completely.
> 
> So what are you proposing that a clock consumer have? The very
> definition of a clock consumer is that it has a clocks property.
> 
To support the cases that the clock tree is defined by clock driver
and device tree together, the "clocks" property could be reasonably
absent in case the parent clock (provider) is being defined in driver
than DT.  And for such clocks, the "clock-names" than "clocks" should
be required to find the clock provider/parent.

I do not like the idea to look for clock with name too much, but I
do not see other way around to support those platforms that have clock
tree definition split in clock driver and device tree. 

> > 
> > Are you sure this is the right thing to do?  If I remember correctly,
> > Grant's position is it should be pretty reasonable to have most of
> > the clock tree defined in clock driver and only define those leaf
> > clocks which are very likely to become the clock providers for other
> > peripherals.
> 
> The minimum is you have to have a provider and consumer. It may be a
> single provider that provides all clocks for a chip. If you don't want a
> provider, then just define a clock-frequency property.
> > 
> > Let me put a terrible example here.  Since clock tree is actually SoC
> > specific, I can reasonably choose to define the entire imx6q clock tree
> > and all the clk lookups for imx6q peripherals in clk-imx6q driver.
> > On imx6q-sabrelite board, the audio codec sgtl5000 uses cko (an imx6q
> > clock available on pad) as the clock source.  That said, I need a board
> > specific clk lookup here, which should be the best user of clock DT
> > bindings.  But sadly, with the current bindings, I can not give the
> > required "clocks" property for sgtl5000 node.
> 
> I don't understand your example. For the sgtl5000 on the sabrelite, you
> would provide a phandle and cell entry that is interpreted as the cko
> pin.

With the bindings here, I need something like below in device tree to
replace the clk lookup registration that is currently done in imx6q
sabrelite specific setup code.  However the problem here is I have cko
defined in clock driver, and thus I can not give phandle to cko in
device tree.  What I suggest is for such cases, we could require
clock-names = "cko" than clocks = <&cko>, and of_clk_get() should also
be able to find the clock with looking for the clk name.

imx6q-sabrelite.dts:

	codec: sgtl5000@0a {
		compatible = "fsl,sgtl5000";
		reg = <0x0a>;
		clocks = <&cko>;
	};

mach-imx6q.c, imx6q_sabrelite_cko1_setup(): 

        cko1 = clk_get_sys(NULL, "cko1");
        clk_register_clkdev(cko1, NULL, "0-000a");

Regards,
Shawn

[GIT PULL] DT clk binding support

[Shawn Guo]

On Sun, May 20, 2012 at 09:18:15PM -0500, Rob Herring wrote:
> > As I said, any clock in the clock tree except root clock is not only
> > a clock provider but also a consumer.  If you define "clocks" as a
> > required property for clock consumers, you are essentially asking users
> > to either define the whole clock tree in the device tree or stay away
> > from device tree completely.
> 
> So what are you proposing that a clock consumer have? The very
> definition of a clock consumer is that it has a clocks property.
> 
To support the cases that the clock tree is defined by clock driver
and device tree together, the "clocks" property could be reasonably
absent in case the parent clock (provider) is being defined in driver
than DT.  And for such clocks, the "clock-names" than "clocks" should
be required to find the clock provider/parent.

I do not like the idea to look for clock with name too much, but I
do not see other way around to support those platforms that have clock
tree definition split in clock driver and device tree. 

> > 
> > Are you sure this is the right thing to do?  If I remember correctly,
> > Grant's position is it should be pretty reasonable to have most of
> > the clock tree defined in clock driver and only define those leaf
> > clocks which are very likely to become the clock providers for other
> > peripherals.
> 
> The minimum is you have to have a provider and consumer. It may be a
> single provider that provides all clocks for a chip. If you don't want a
> provider, then just define a clock-frequency property.
> > 
> > Let me put a terrible example here.  Since clock tree is actually SoC
> > specific, I can reasonably choose to define the entire imx6q clock tree
> > and all the clk lookups for imx6q peripherals in clk-imx6q driver.
> > On imx6q-sabrelite board, the audio codec sgtl5000 uses cko (an imx6q
> > clock available on pad) as the clock source.  That said, I need a board
> > specific clk lookup here, which should be the best user of clock DT
> > bindings.  But sadly, with the current bindings, I can not give the
> > required "clocks" property for sgtl5000 node.
> 
> I don't understand your example. For the sgtl5000 on the sabrelite, you
> would provide a phandle and cell entry that is interpreted as the cko
> pin.

With the bindings here, I need something like below in device tree to
replace the clk lookup registration that is currently done in imx6q
sabrelite specific setup code.  However the problem here is I have cko
defined in clock driver, and thus I can not give phandle to cko in
device tree.  What I suggest is for such cases, we could require
clock-names = "cko" than clocks = <&cko>, and of_clk_get() should also
be able to find the clock with looking for the clk name.

imx6q-sabrelite.dts:

	codec: sgtl5000 at 0a {
		compatible = "fsl,sgtl5000";
		reg = <0x0a>;
		clocks = <&cko>;
	};

mach-imx6q.c, imx6q_sabrelite_cko1_setup(): 

        cko1 = clk_get_sys(NULL, "cko1");
        clk_register_clkdev(cko1, NULL, "0-000a");

Regards,
Shawn

Re: [GIT PULL] DT clk binding support

[Rob Herring]

On 05/21/2012 01:49 AM, Shawn Guo wrote:
> On Sun, May 20, 2012 at 09:18:15PM -0500, Rob Herring wrote:
>>> As I said, any clock in the clock tree except root clock is not only
>>> a clock provider but also a consumer.  If you define "clocks" as a
>>> required property for clock consumers, you are essentially asking users
>>> to either define the whole clock tree in the device tree or stay away
>>> from device tree completely.
>>
>> So what are you proposing that a clock consumer have? The very
>> definition of a clock consumer is that it has a clocks property.
>>
> To support the cases that the clock tree is defined by clock driver
> and device tree together, the "clocks" property could be reasonably
> absent in case the parent clock (provider) is being defined in driver
> than DT.  And for such clocks, the "clock-names" than "clocks" should
> be required to find the clock provider/parent.
> 
> I do not like the idea to look for clock with name too much, but I
> do not see other way around to support those platforms that have clock
> tree definition split in clock driver and device tree. 
> 
>>>
>>> Are you sure this is the right thing to do?  If I remember correctly,
>>> Grant's position is it should be pretty reasonable to have most of
>>> the clock tree defined in clock driver and only define those leaf
>>> clocks which are very likely to become the clock providers for other
>>> peripherals.
>>
>> The minimum is you have to have a provider and consumer. It may be a
>> single provider that provides all clocks for a chip. If you don't want a
>> provider, then just define a clock-frequency property.
>>>
>>> Let me put a terrible example here.  Since clock tree is actually SoC
>>> specific, I can reasonably choose to define the entire imx6q clock tree
>>> and all the clk lookups for imx6q peripherals in clk-imx6q driver.
>>> On imx6q-sabrelite board, the audio codec sgtl5000 uses cko (an imx6q
>>> clock available on pad) as the clock source.  That said, I need a board
>>> specific clk lookup here, which should be the best user of clock DT
>>> bindings.  But sadly, with the current bindings, I can not give the
>>> required "clocks" property for sgtl5000 node.
>>
>> I don't understand your example. For the sgtl5000 on the sabrelite, you
>> would provide a phandle and cell entry that is interpreted as the cko
>> pin.
> 
> With the bindings here, I need something like below in device tree to
> replace the clk lookup registration that is currently done in imx6q
> sabrelite specific setup code.  However the problem here is I have cko
> defined in clock driver, and thus I can not give phandle to cko in
> device tree.  What I suggest is for such cases, we could require
> clock-names = "cko" than clocks = <&cko>, and of_clk_get() should also
> be able to find the clock with looking for the clk name.
> 
> imx6q-sabrelite.dts:
> 
> 	codec: sgtl5000@0a {
> 		compatible = "fsl,sgtl5000";
> 		reg = <0x0a>;
> 		clocks = <&cko>;
> 	};
> 
> mach-imx6q.c, imx6q_sabrelite_cko1_setup(): 
> 
>         cko1 = clk_get_sys(NULL, "cko1");
>         clk_register_clkdev(cko1, NULL, "0-000a");
> 

What!? This is a terrible abuse/hack of the clock binding and is in no
way what was intended. You cannot use half of the clock binding. You
have to have a provider. The primary binding is a phandle reference.
clock-names is just auxiliary data.

Rob

[GIT PULL] DT clk binding support

[Rob Herring]

On 05/21/2012 01:49 AM, Shawn Guo wrote:
> On Sun, May 20, 2012 at 09:18:15PM -0500, Rob Herring wrote:
>>> As I said, any clock in the clock tree except root clock is not only
>>> a clock provider but also a consumer.  If you define "clocks" as a
>>> required property for clock consumers, you are essentially asking users
>>> to either define the whole clock tree in the device tree or stay away
>>> from device tree completely.
>>
>> So what are you proposing that a clock consumer have? The very
>> definition of a clock consumer is that it has a clocks property.
>>
> To support the cases that the clock tree is defined by clock driver
> and device tree together, the "clocks" property could be reasonably
> absent in case the parent clock (provider) is being defined in driver
> than DT.  And for such clocks, the "clock-names" than "clocks" should
> be required to find the clock provider/parent.
> 
> I do not like the idea to look for clock with name too much, but I
> do not see other way around to support those platforms that have clock
> tree definition split in clock driver and device tree. 
> 
>>>
>>> Are you sure this is the right thing to do?  If I remember correctly,
>>> Grant's position is it should be pretty reasonable to have most of
>>> the clock tree defined in clock driver and only define those leaf
>>> clocks which are very likely to become the clock providers for other
>>> peripherals.
>>
>> The minimum is you have to have a provider and consumer. It may be a
>> single provider that provides all clocks for a chip. If you don't want a
>> provider, then just define a clock-frequency property.
>>>
>>> Let me put a terrible example here.  Since clock tree is actually SoC
>>> specific, I can reasonably choose to define the entire imx6q clock tree
>>> and all the clk lookups for imx6q peripherals in clk-imx6q driver.
>>> On imx6q-sabrelite board, the audio codec sgtl5000 uses cko (an imx6q
>>> clock available on pad) as the clock source.  That said, I need a board
>>> specific clk lookup here, which should be the best user of clock DT
>>> bindings.  But sadly, with the current bindings, I can not give the
>>> required "clocks" property for sgtl5000 node.
>>
>> I don't understand your example. For the sgtl5000 on the sabrelite, you
>> would provide a phandle and cell entry that is interpreted as the cko
>> pin.
> 
> With the bindings here, I need something like below in device tree to
> replace the clk lookup registration that is currently done in imx6q
> sabrelite specific setup code.  However the problem here is I have cko
> defined in clock driver, and thus I can not give phandle to cko in
> device tree.  What I suggest is for such cases, we could require
> clock-names = "cko" than clocks = <&cko>, and of_clk_get() should also
> be able to find the clock with looking for the clk name.
> 
> imx6q-sabrelite.dts:
> 
> 	codec: sgtl5000 at 0a {
> 		compatible = "fsl,sgtl5000";
> 		reg = <0x0a>;
> 		clocks = <&cko>;
> 	};
> 
> mach-imx6q.c, imx6q_sabrelite_cko1_setup(): 
> 
>         cko1 = clk_get_sys(NULL, "cko1");
>         clk_register_clkdev(cko1, NULL, "0-000a");
> 

What!? This is a terrible abuse/hack of the clock binding and is in no
way what was intended. You cannot use half of the clock binding. You
have to have a provider. The primary binding is a phandle reference.
clock-names is just auxiliary data.

Rob

Re: [GIT PULL] DT clk binding support

[Shawn Guo]

On Mon, May 21, 2012 at 01:30:59PM -0500, Rob Herring wrote:
> > With the bindings here, I need something like below in device tree to
> > replace the clk lookup registration that is currently done in imx6q
> > sabrelite specific setup code.  However the problem here is I have cko
> > defined in clock driver, and thus I can not give phandle to cko in
> > device tree.  What I suggest is for such cases, we could require
> > clock-names = "cko" than clocks = <&cko>, and of_clk_get() should also
> > be able to find the clock with looking for the clk name.
> > 
> > imx6q-sabrelite.dts:
> > 
> > 	codec: sgtl5000@0a {
> > 		compatible = "fsl,sgtl5000";
> > 		reg = <0x0a>;
> > 		clocks = <&cko>;
> > 	};
> > 
> > mach-imx6q.c, imx6q_sabrelite_cko1_setup(): 
> > 
> >         cko1 = clk_get_sys(NULL, "cko1");
> >         clk_register_clkdev(cko1, NULL, "0-000a");
> > 
> 
> What!? This is a terrible abuse/hack of the clock binding and is in no
> way what was intended. You cannot use half of the clock binding. You
> have to have a provider. The primary binding is a phandle reference.
> clock-names is just auxiliary data.
> 
Well, on conversation [1], it seems people agreed that for those huge
clock tree, only leaf clocks should be exposed in device tree.  Then
please help me understand how we could do that with the current binding
design, considering those leaf clocks are consumers of their parent
clocks while being provider to the leaf clock the parent clock is not
exposed in device tree.

Regards,
Shawn

[1] http://thread.gmane.org/gmane.linux.ports.arm.kernel/139414/focus=1216423

[GIT PULL] DT clk binding support

[Shawn Guo]

On Mon, May 21, 2012 at 01:30:59PM -0500, Rob Herring wrote:
> > With the bindings here, I need something like below in device tree to
> > replace the clk lookup registration that is currently done in imx6q
> > sabrelite specific setup code.  However the problem here is I have cko
> > defined in clock driver, and thus I can not give phandle to cko in
> > device tree.  What I suggest is for such cases, we could require
> > clock-names = "cko" than clocks = <&cko>, and of_clk_get() should also
> > be able to find the clock with looking for the clk name.
> > 
> > imx6q-sabrelite.dts:
> > 
> > 	codec: sgtl5000 at 0a {
> > 		compatible = "fsl,sgtl5000";
> > 		reg = <0x0a>;
> > 		clocks = <&cko>;
> > 	};
> > 
> > mach-imx6q.c, imx6q_sabrelite_cko1_setup(): 
> > 
> >         cko1 = clk_get_sys(NULL, "cko1");
> >         clk_register_clkdev(cko1, NULL, "0-000a");
> > 
> 
> What!? This is a terrible abuse/hack of the clock binding and is in no
> way what was intended. You cannot use half of the clock binding. You
> have to have a provider. The primary binding is a phandle reference.
> clock-names is just auxiliary data.
> 
Well, on conversation [1], it seems people agreed that for those huge
clock tree, only leaf clocks should be exposed in device tree.  Then
please help me understand how we could do that with the current binding
design, considering those leaf clocks are consumers of their parent
clocks while being provider to the leaf clock the parent clock is not
exposed in device tree.

Regards,
Shawn

[1] http://thread.gmane.org/gmane.linux.ports.arm.kernel/139414/focus=1216423

Re: [GIT PULL] DT clk binding support

[Rob Herring]

On 05/21/2012 06:26 PM, Shawn Guo wrote:
> On Mon, May 21, 2012 at 01:30:59PM -0500, Rob Herring wrote:
>>> With the bindings here, I need something like below in device tree to
>>> replace the clk lookup registration that is currently done in imx6q
>>> sabrelite specific setup code.  However the problem here is I have cko
>>> defined in clock driver, and thus I can not give phandle to cko in
>>> device tree.  What I suggest is for such cases, we could require
>>> clock-names = "cko" than clocks = <&cko>, and of_clk_get() should also
>>> be able to find the clock with looking for the clk name.
>>>
>>> imx6q-sabrelite.dts:
>>>
>>> 	codec: sgtl5000@0a {
>>> 		compatible = "fsl,sgtl5000";
>>> 		reg = <0x0a>;
>>> 		clocks = <&cko>;
>>> 	};
>>>
>>> mach-imx6q.c, imx6q_sabrelite_cko1_setup(): 
>>>
>>>         cko1 = clk_get_sys(NULL, "cko1");
>>>         clk_register_clkdev(cko1, NULL, "0-000a");
>>>
>>
>> What!? This is a terrible abuse/hack of the clock binding and is in no
>> way what was intended. You cannot use half of the clock binding. You
>> have to have a provider. The primary binding is a phandle reference.
>> clock-names is just auxiliary data.
>>
> Well, on conversation [1], it seems people agreed that for those huge
> clock tree, only leaf clocks should be exposed in device tree.  Then
> please help me understand how we could do that with the current binding
> design, considering those leaf clocks are consumers of their parent
> clocks while being provider to the leaf clock the parent clock is not
> exposed in device tree.

You are mis-interpreting things.

As Grant states: "This proposed binding is only about one thing:
attaching clock providers to clock consumers." This means you have to
have at least a single provider and a single consumer defined in the DT.

By only exposing leaf nodes of the clocks, that means only exposing a
single DT node for the SOC clocks with a whole bunch of outputs (i.e.
the leaf clocks). In the imx case, this would be a single node for the
CCM with the dozens clocks the CCM outputs. In this case you would not
expose all the individual muxes, gates, and dividers of the CCM in the
DT. You still have to describe the connection between the CCM and a h/w
block. I'm not sure what you want here. Based on our prior
conversations, I thought you wanted to break out every single clock as a
separate DT node and have generic divider, mux, and gate bindings. Now
you are arguing for the opposite.

Rob

> Regards,
> Shawn
> 
> [1] http://thread.gmane.org/gmane.linux.ports.arm.kernel/139414/focus=1216423

[GIT PULL] DT clk binding support

[Rob Herring]

On 05/21/2012 06:26 PM, Shawn Guo wrote:
> On Mon, May 21, 2012 at 01:30:59PM -0500, Rob Herring wrote:
>>> With the bindings here, I need something like below in device tree to
>>> replace the clk lookup registration that is currently done in imx6q
>>> sabrelite specific setup code.  However the problem here is I have cko
>>> defined in clock driver, and thus I can not give phandle to cko in
>>> device tree.  What I suggest is for such cases, we could require
>>> clock-names = "cko" than clocks = <&cko>, and of_clk_get() should also
>>> be able to find the clock with looking for the clk name.
>>>
>>> imx6q-sabrelite.dts:
>>>
>>> 	codec: sgtl5000 at 0a {
>>> 		compatible = "fsl,sgtl5000";
>>> 		reg = <0x0a>;
>>> 		clocks = <&cko>;
>>> 	};
>>>
>>> mach-imx6q.c, imx6q_sabrelite_cko1_setup(): 
>>>
>>>         cko1 = clk_get_sys(NULL, "cko1");
>>>         clk_register_clkdev(cko1, NULL, "0-000a");
>>>
>>
>> What!? This is a terrible abuse/hack of the clock binding and is in no
>> way what was intended. You cannot use half of the clock binding. You
>> have to have a provider. The primary binding is a phandle reference.
>> clock-names is just auxiliary data.
>>
> Well, on conversation [1], it seems people agreed that for those huge
> clock tree, only leaf clocks should be exposed in device tree.  Then
> please help me understand how we could do that with the current binding
> design, considering those leaf clocks are consumers of their parent
> clocks while being provider to the leaf clock the parent clock is not
> exposed in device tree.

You are mis-interpreting things.

As Grant states: "This proposed binding is only about one thing:
attaching clock providers to clock consumers." This means you have to
have at least a single provider and a single consumer defined in the DT.

By only exposing leaf nodes of the clocks, that means only exposing a
single DT node for the SOC clocks with a whole bunch of outputs (i.e.
the leaf clocks). In the imx case, this would be a single node for the
CCM with the dozens clocks the CCM outputs. In this case you would not
expose all the individual muxes, gates, and dividers of the CCM in the
DT. You still have to describe the connection between the CCM and a h/w
block. I'm not sure what you want here. Based on our prior
conversations, I thought you wanted to break out every single clock as a
separate DT node and have generic divider, mux, and gate bindings. Now
you are arguing for the opposite.

Rob

> Regards,
> Shawn
> 
> [1] http://thread.gmane.org/gmane.linux.ports.arm.kernel/139414/focus=1216423

Re: [GIT PULL] DT clk binding support

[Shawn Guo]

On Mon, May 21, 2012 at 06:52:37PM -0500, Rob Herring wrote:
> As Grant states: "This proposed binding is only about one thing:
> attaching clock providers to clock consumers." This means you have to
> have at least a single provider and a single consumer defined in the DT.
> 
I just read through Grant's comments over again.  I agree with the
statement which implicitly requires the clk provider defined in DT.
However, for some case, this provider in DT is just a skeleton which
is backed by clock driver where the provider is actually defined.

Looking at Grant's comment below, the second option is also to match
the clock in driver just using name.  The only difference to my
proposal is the name here is given by the argument of phandle pointing
to that skeleton provider node.

I'm fine with that.  So go ahead with your bindings.

Regards,
Shawn

On Wed, Nov 16, 2011 at 03:12:50PM -0700, Grant Likely wrote:
> I'm really not convinced that it is a good idea to break out the
> entire clock tree into one node per struct clk.  To begin with, that
> looks to be very centric around the current 'struct clk' Linux
> abstraction, which is potentially in flux.  Also, looking at Sascha's
> initial RFC for describing the clock tree, I see cases where it looks
> like a clock nexus node really makes sense.  For instance, the
> 'divider-ipg <at> 0x53fd4014' node which has a list of child nodes
> which merely provide a register offset and shift value
> (reg=0x53fd4068..0x53fd4084, shift=0x0..0xf).  It would be natural to
> instead encode that as part of the clock reference, or map it directly
> from the clock reference (ie, assign names to each of the clocks, and
> let the clock provider driver match up the name to the reg offset &
> shift values).
>
> I had originally thought that it would be better to use names directly
> for references to clocks (ie. clock = <phandle>,"name") , but after
> actually playing with it and looking at the existing DT conventions,
> I've reverted to cell values for the arguments and a separate set of
> clock-{input,output}-name properties for attaching meaningful names,
> just like we decided to do for assigning names to 'reg' properties.

[GIT PULL] DT clk binding support

[Shawn Guo]

On Mon, May 21, 2012 at 06:52:37PM -0500, Rob Herring wrote:
> As Grant states: "This proposed binding is only about one thing:
> attaching clock providers to clock consumers." This means you have to
> have at least a single provider and a single consumer defined in the DT.
> 
I just read through Grant's comments over again.  I agree with the
statement which implicitly requires the clk provider defined in DT.
However, for some case, this provider in DT is just a skeleton which
is backed by clock driver where the provider is actually defined.

Looking at Grant's comment below, the second option is also to match
the clock in driver just using name.  The only difference to my
proposal is the name here is given by the argument of phandle pointing
to that skeleton provider node.

I'm fine with that.  So go ahead with your bindings.

Regards,
Shawn

On Wed, Nov 16, 2011 at 03:12:50PM -0700, Grant Likely wrote:
> I'm really not convinced that it is a good idea to break out the
> entire clock tree into one node per struct clk.  To begin with, that
> looks to be very centric around the current 'struct clk' Linux
> abstraction, which is potentially in flux.  Also, looking at Sascha's
> initial RFC for describing the clock tree, I see cases where it looks
> like a clock nexus node really makes sense.  For instance, the
> 'divider-ipg <at> 0x53fd4014' node which has a list of child nodes
> which merely provide a register offset and shift value
> (reg=0x53fd4068..0x53fd4084, shift=0x0..0xf).  It would be natural to
> instead encode that as part of the clock reference, or map it directly
> from the clock reference (ie, assign names to each of the clocks, and
> let the clock provider driver match up the name to the reg offset &
> shift values).
>
> I had originally thought that it would be better to use names directly
> for references to clocks (ie. clock = <phandle>,"name") , but after
> actually playing with it and looking at the existing DT conventions,
> I've reverted to cell values for the arguments and a separate set of
> clock-{input,output}-name properties for attaching meaningful names,
> just like we decided to do for assigning names to 'reg' properties.

Re: [GIT PULL] DT clk binding support

[Stephen Boyd]

On 05/21/12 19:15, Shawn Guo wrote:
> On Mon, May 21, 2012 at 06:52:37PM -0500, Rob Herring wrote:
>> As Grant states: "This proposed binding is only about one thing:
>> attaching clock providers to clock consumers." This means you have to
>> have at least a single provider and a single consumer defined in the DT.
>>
> I just read through Grant's comments over again.  I agree with the
> statement which implicitly requires the clk provider defined in DT.
> However, for some case, this provider in DT is just a skeleton which
> is backed by clock driver where the provider is actually defined.
>
> Looking at Grant's comment below, the second option is also to match
> the clock in driver just using name.  The only difference to my
> proposal is the name here is given by the argument of phandle pointing
> to that skeleton provider node.
>
> I'm fine with that.  So go ahead with your bindings.
>

Can we do what the regulator framework has done and have a common
binding of <connection_name>-clk = <&phandle>? Something like:

    core-clk = <&uart3_clk>

and then have clk_get() use the of node of the device passed in to find
a property named %s-clk and find the clock with the matching phandle.

This looks like it's trying to cover both the end consumers (uart uses
uart3_clk) and the internal clock tree consumers (a crystal oscillator
connects to a PLL or a mux has multiple parents). We can certainly use
these bindings for muxes and internal parent-child relationships but I
would prefer we use different bindings for consumer bindings that match
what regulators do today.

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[GIT PULL] DT clk binding support

[Stephen Boyd]

On 05/21/12 19:15, Shawn Guo wrote:
> On Mon, May 21, 2012 at 06:52:37PM -0500, Rob Herring wrote:
>> As Grant states: "This proposed binding is only about one thing:
>> attaching clock providers to clock consumers." This means you have to
>> have at least a single provider and a single consumer defined in the DT.
>>
> I just read through Grant's comments over again.  I agree with the
> statement which implicitly requires the clk provider defined in DT.
> However, for some case, this provider in DT is just a skeleton which
> is backed by clock driver where the provider is actually defined.
>
> Looking at Grant's comment below, the second option is also to match
> the clock in driver just using name.  The only difference to my
> proposal is the name here is given by the argument of phandle pointing
> to that skeleton provider node.
>
> I'm fine with that.  So go ahead with your bindings.
>

Can we do what the regulator framework has done and have a common
binding of <connection_name>-clk = <&phandle>? Something like:

    core-clk = <&uart3_clk>

and then have clk_get() use the of node of the device passed in to find
a property named %s-clk and find the clock with the matching phandle.

This looks like it's trying to cover both the end consumers (uart uses
uart3_clk) and the internal clock tree consumers (a crystal oscillator
connects to a PLL or a mux has multiple parents). We can certainly use
these bindings for muxes and internal parent-child relationships but I
would prefer we use different bindings for consumer bindings that match
what regulators do today.

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [GIT PULL] DT clk binding support

[Rob Herring]

On 05/21/2012 11:17 PM, Stephen Boyd wrote:
> On 05/21/12 19:15, Shawn Guo wrote:
>> On Mon, May 21, 2012 at 06:52:37PM -0500, Rob Herring wrote:
>>> As Grant states: "This proposed binding is only about one thing:
>>> attaching clock providers to clock consumers." This means you have to
>>> have at least a single provider and a single consumer defined in the DT.
>>>
>> I just read through Grant's comments over again.  I agree with the
>> statement which implicitly requires the clk provider defined in DT.
>> However, for some case, this provider in DT is just a skeleton which
>> is backed by clock driver where the provider is actually defined.
>>
>> Looking at Grant's comment below, the second option is also to match
>> the clock in driver just using name.  The only difference to my
>> proposal is the name here is given by the argument of phandle pointing
>> to that skeleton provider node.
>>
>> I'm fine with that.  So go ahead with your bindings.
>>
> 
> Can we do what the regulator framework has done and have a common
> binding of <connection_name>-clk = <&phandle>? Something like:
> 
>     core-clk = <&uart3_clk>
> 
> and then have clk_get() use the of node of the device passed in to find
> a property named %s-clk and find the clock with the matching phandle.

Sigh... That is what we had in previous versions from over a year ago
and we moved away from that approach. The current binding has been
reviewed multiple times in the last 6 months...

The current approach is aligned with how interrupts are handled (with
the addition of a phandle). I think not having per clock property names
is easier to parse and easier to document.

> This looks like it's trying to cover both the end consumers (uart uses
> uart3_clk) and the internal clock tree consumers (a crystal oscillator
> connects to a PLL or a mux has multiple parents). We can certainly use
> these bindings for muxes and internal parent-child relationships but I
> would prefer we use different bindings for consumer bindings that match
> what regulators do today.

The binding supports either defining every last internal clock or just
the leaf clocks. I took the former route on highbank since I don't have
a lot of clocks. If I was doing imx or omap for example, I'd probably
just define all the clock controller outputs.

Rob

[GIT PULL] DT clk binding support

[Rob Herring]

On 05/21/2012 11:17 PM, Stephen Boyd wrote:
> On 05/21/12 19:15, Shawn Guo wrote:
>> On Mon, May 21, 2012 at 06:52:37PM -0500, Rob Herring wrote:
>>> As Grant states: "This proposed binding is only about one thing:
>>> attaching clock providers to clock consumers." This means you have to
>>> have at least a single provider and a single consumer defined in the DT.
>>>
>> I just read through Grant's comments over again.  I agree with the
>> statement which implicitly requires the clk provider defined in DT.
>> However, for some case, this provider in DT is just a skeleton which
>> is backed by clock driver where the provider is actually defined.
>>
>> Looking at Grant's comment below, the second option is also to match
>> the clock in driver just using name.  The only difference to my
>> proposal is the name here is given by the argument of phandle pointing
>> to that skeleton provider node.
>>
>> I'm fine with that.  So go ahead with your bindings.
>>
> 
> Can we do what the regulator framework has done and have a common
> binding of <connection_name>-clk = <&phandle>? Something like:
> 
>     core-clk = <&uart3_clk>
> 
> and then have clk_get() use the of node of the device passed in to find
> a property named %s-clk and find the clock with the matching phandle.

Sigh... That is what we had in previous versions from over a year ago
and we moved away from that approach. The current binding has been
reviewed multiple times in the last 6 months...

The current approach is aligned with how interrupts are handled (with
the addition of a phandle). I think not having per clock property names
is easier to parse and easier to document.

> This looks like it's trying to cover both the end consumers (uart uses
> uart3_clk) and the internal clock tree consumers (a crystal oscillator
> connects to a PLL or a mux has multiple parents). We can certainly use
> these bindings for muxes and internal parent-child relationships but I
> would prefer we use different bindings for consumer bindings that match
> what regulators do today.

The binding supports either defining every last internal clock or just
the leaf clocks. I took the former route on highbank since I don't have
a lot of clocks. If I was doing imx or omap for example, I'd probably
just define all the clock controller outputs.

Rob

Re: [GIT PULL] DT clk binding support

[Saravana Kannan]

On Tue, May 22, 2012 6:52 am, Rob Herring wrote:
> On 05/21/2012 11:17 PM, Stephen Boyd wrote:
>> On 05/21/12 19:15, Shawn Guo wrote:
>>> On Mon, May 21, 2012 at 06:52:37PM -0500, Rob Herring wrote:
>>>> As Grant states: "This proposed binding is only about one thing:
>>>> attaching clock providers to clock consumers." This means you have to
>>>> have at least a single provider and a single consumer defined in the
>>>> DT.
>>>>
>>> I just read through Grant's comments over again.  I agree with the
>>> statement which implicitly requires the clk provider defined in DT.
>>> However, for some case, this provider in DT is just a skeleton which
>>> is backed by clock driver where the provider is actually defined.
>>>
>>> Looking at Grant's comment below, the second option is also to match
>>> the clock in driver just using name.  The only difference to my
>>> proposal is the name here is given by the argument of phandle pointing
>>> to that skeleton provider node.
>>>
>>> I'm fine with that.  So go ahead with your bindings.
>>>
>>
>> Can we do what the regulator framework has done and have a common
>> binding of <connection_name>-clk = <&phandle>? Something like:
>>
>>     core-clk = <&uart3_clk>
>>
>> and then have clk_get() use the of node of the device passed in to find
>> a property named %s-clk and find the clock with the matching phandle.
>
> Sigh... That is what we had in previous versions from over a year ago
> and we moved away from that approach. The current binding has been
> reviewed multiple times in the last 6 months...
>
> The current approach is aligned with how interrupts are handled (with
> the addition of a phandle). I think not having per clock property names
> is easier to parse and easier to document.
>
>> This looks like it's trying to cover both the end consumers (uart uses
>> uart3_clk) and the internal clock tree consumers (a crystal oscillator
>> connects to a PLL or a mux has multiple parents). We can certainly use
>> these bindings for muxes and internal parent-child relationships but I
>> would prefer we use different bindings for consumer bindings that match
>> what regulators do today.
>
> The binding supports either defining every last internal clock or just
> the leaf clocks. I took the former route on highbank since I don't have
> a lot of clocks. If I was doing imx or omap for example, I'd probably
> just define all the clock controller outputs.
>

If only the leaf nodes are defined in DT, then how is the clock platform
driver implementer supposed to instantiate the rest of the tree and
connect it up with the partial list of clocks in DT? So, they have to
switch back and forth between DT and the .c file which defines the rest
and make sure the parent<->child names match?

To me it looks that it might better to decouple the description of the
clock HW from the mapping of a clock leaf to a consumer device. If we just
use a string to identify the clock that's consumed by a device, we can
achieve this decoupling at a clean boundary -- clock consumers devices
(UART) vs clock producer devices (clock controller in the SoC, in a PMIC,
audio codec, etc).

With the decoupling, we don't have the inconsistency of having some of the
clocks of a clock producer device incompletely defined in DT and the rest
of the clocks of the same clock producer device hard coded in the kernel.
So, you either put your entire clock tree in the SoC in the DT or put all
of it in the kernel but you aren't forced to put just some of them in the
DT just to get DT working. I see no benefit in defining only some of the
clocks in DT -- it just adds more confusion in the clock tree definition.
What am I missing?

Thanks,
Saravana

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[GIT PULL] DT clk binding support

[Saravana Kannan]

On Tue, May 22, 2012 6:52 am, Rob Herring wrote:
> On 05/21/2012 11:17 PM, Stephen Boyd wrote:
>> On 05/21/12 19:15, Shawn Guo wrote:
>>> On Mon, May 21, 2012 at 06:52:37PM -0500, Rob Herring wrote:
>>>> As Grant states: "This proposed binding is only about one thing:
>>>> attaching clock providers to clock consumers." This means you have to
>>>> have at least a single provider and a single consumer defined in the
>>>> DT.
>>>>
>>> I just read through Grant's comments over again.  I agree with the
>>> statement which implicitly requires the clk provider defined in DT.
>>> However, for some case, this provider in DT is just a skeleton which
>>> is backed by clock driver where the provider is actually defined.
>>>
>>> Looking at Grant's comment below, the second option is also to match
>>> the clock in driver just using name.  The only difference to my
>>> proposal is the name here is given by the argument of phandle pointing
>>> to that skeleton provider node.
>>>
>>> I'm fine with that.  So go ahead with your bindings.
>>>
>>
>> Can we do what the regulator framework has done and have a common
>> binding of <connection_name>-clk = <&phandle>? Something like:
>>
>>     core-clk = <&uart3_clk>
>>
>> and then have clk_get() use the of node of the device passed in to find
>> a property named %s-clk and find the clock with the matching phandle.
>
> Sigh... That is what we had in previous versions from over a year ago
> and we moved away from that approach. The current binding has been
> reviewed multiple times in the last 6 months...
>
> The current approach is aligned with how interrupts are handled (with
> the addition of a phandle). I think not having per clock property names
> is easier to parse and easier to document.
>
>> This looks like it's trying to cover both the end consumers (uart uses
>> uart3_clk) and the internal clock tree consumers (a crystal oscillator
>> connects to a PLL or a mux has multiple parents). We can certainly use
>> these bindings for muxes and internal parent-child relationships but I
>> would prefer we use different bindings for consumer bindings that match
>> what regulators do today.
>
> The binding supports either defining every last internal clock or just
> the leaf clocks. I took the former route on highbank since I don't have
> a lot of clocks. If I was doing imx or omap for example, I'd probably
> just define all the clock controller outputs.
>

If only the leaf nodes are defined in DT, then how is the clock platform
driver implementer supposed to instantiate the rest of the tree and
connect it up with the partial list of clocks in DT? So, they have to
switch back and forth between DT and the .c file which defines the rest
and make sure the parent<->child names match?

To me it looks that it might better to decouple the description of the
clock HW from the mapping of a clock leaf to a consumer device. If we just
use a string to identify the clock that's consumed by a device, we can
achieve this decoupling at a clean boundary -- clock consumers devices
(UART) vs clock producer devices (clock controller in the SoC, in a PMIC,
audio codec, etc).

With the decoupling, we don't have the inconsistency of having some of the
clocks of a clock producer device incompletely defined in DT and the rest
of the clocks of the same clock producer device hard coded in the kernel.
So, you either put your entire clock tree in the SoC in the DT or put all
of it in the kernel but you aren't forced to put just some of them in the
DT just to get DT working. I see no benefit in defining only some of the
clocks in DT -- it just adds more confusion in the clock tree definition.
What am I missing?

Thanks,
Saravana

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [GIT PULL] DT clk binding support

[Rob Herring]

On 05/22/2012 08:38 PM, Saravana Kannan wrote:
> 
> On Tue, May 22, 2012 6:52 am, Rob Herring wrote:
>> On 05/21/2012 11:17 PM, Stephen Boyd wrote:
>>> On 05/21/12 19:15, Shawn Guo wrote:
>>>> On Mon, May 21, 2012 at 06:52:37PM -0500, Rob Herring wrote:
>>>>> As Grant states: "This proposed binding is only about one thing:
>>>>> attaching clock providers to clock consumers." This means you have to
>>>>> have at least a single provider and a single consumer defined in the
>>>>> DT.
>>>>>
>>>> I just read through Grant's comments over again.  I agree with the
>>>> statement which implicitly requires the clk provider defined in DT.
>>>> However, for some case, this provider in DT is just a skeleton which
>>>> is backed by clock driver where the provider is actually defined.
>>>>
>>>> Looking at Grant's comment below, the second option is also to match
>>>> the clock in driver just using name.  The only difference to my
>>>> proposal is the name here is given by the argument of phandle pointing
>>>> to that skeleton provider node.
>>>>
>>>> I'm fine with that.  So go ahead with your bindings.
>>>>
>>>
>>> Can we do what the regulator framework has done and have a common
>>> binding of <connection_name>-clk = <&phandle>? Something like:
>>>
>>>     core-clk = <&uart3_clk>
>>>
>>> and then have clk_get() use the of node of the device passed in to find
>>> a property named %s-clk and find the clock with the matching phandle.
>>
>> Sigh... That is what we had in previous versions from over a year ago
>> and we moved away from that approach. The current binding has been
>> reviewed multiple times in the last 6 months...
>>
>> The current approach is aligned with how interrupts are handled (with
>> the addition of a phandle). I think not having per clock property names
>> is easier to parse and easier to document.
>>
>>> This looks like it's trying to cover both the end consumers (uart uses
>>> uart3_clk) and the internal clock tree consumers (a crystal oscillator
>>> connects to a PLL or a mux has multiple parents). We can certainly use
>>> these bindings for muxes and internal parent-child relationships but I
>>> would prefer we use different bindings for consumer bindings that match
>>> what regulators do today.
>>
>> The binding supports either defining every last internal clock or just
>> the leaf clocks. I took the former route on highbank since I don't have
>> a lot of clocks. If I was doing imx or omap for example, I'd probably
>> just define all the clock controller outputs.
>>
> 
> If only the leaf nodes are defined in DT, then how is the clock platform
> driver implementer supposed to instantiate the rest of the tree and
> connect it up with the partial list of clocks in DT? So, they have to
> switch back and forth between DT and the .c file which defines the rest
> and make sure the parent<->child names match?
> 
> To me it looks that it might better to decouple the description of the
> clock HW from the mapping of a clock leaf to a consumer device. If we just
> use a string to identify the clock that's consumed by a device, we can
> achieve this decoupling at a clean boundary -- clock consumers devices
> (UART) vs clock producer devices (clock controller in the SoC, in a PMIC,
> audio codec, etc).
> 
> With the decoupling, we don't have the inconsistency of having some of the
> clocks of a clock producer device incompletely defined in DT and the rest
> of the clocks of the same clock producer device hard coded in the kernel.
> So, you either put your entire clock tree in the SoC in the DT or put all
> of it in the kernel but you aren't forced to put just some of them in the
> DT just to get DT working. I see no benefit in defining only some of the
> clocks in DT -- it just adds more confusion in the clock tree definition.
> What am I missing?

I fail to see what would need changing in the binding itself. The
binding just describes connections. Whether that is a connection to a
clock controller node to a device or a clock gate/mux/divider node to a
device is really beyond the clock binding. This is really just policy.
You are free to put no clocks in DT, all clocks, or a nexus of clocks.

Rob

[GIT PULL] DT clk binding support

[Rob Herring]

On 05/22/2012 08:38 PM, Saravana Kannan wrote:
> 
> On Tue, May 22, 2012 6:52 am, Rob Herring wrote:
>> On 05/21/2012 11:17 PM, Stephen Boyd wrote:
>>> On 05/21/12 19:15, Shawn Guo wrote:
>>>> On Mon, May 21, 2012 at 06:52:37PM -0500, Rob Herring wrote:
>>>>> As Grant states: "This proposed binding is only about one thing:
>>>>> attaching clock providers to clock consumers." This means you have to
>>>>> have at least a single provider and a single consumer defined in the
>>>>> DT.
>>>>>
>>>> I just read through Grant's comments over again.  I agree with the
>>>> statement which implicitly requires the clk provider defined in DT.
>>>> However, for some case, this provider in DT is just a skeleton which
>>>> is backed by clock driver where the provider is actually defined.
>>>>
>>>> Looking at Grant's comment below, the second option is also to match
>>>> the clock in driver just using name.  The only difference to my
>>>> proposal is the name here is given by the argument of phandle pointing
>>>> to that skeleton provider node.
>>>>
>>>> I'm fine with that.  So go ahead with your bindings.
>>>>
>>>
>>> Can we do what the regulator framework has done and have a common
>>> binding of <connection_name>-clk = <&phandle>? Something like:
>>>
>>>     core-clk = <&uart3_clk>
>>>
>>> and then have clk_get() use the of node of the device passed in to find
>>> a property named %s-clk and find the clock with the matching phandle.
>>
>> Sigh... That is what we had in previous versions from over a year ago
>> and we moved away from that approach. The current binding has been
>> reviewed multiple times in the last 6 months...
>>
>> The current approach is aligned with how interrupts are handled (with
>> the addition of a phandle). I think not having per clock property names
>> is easier to parse and easier to document.
>>
>>> This looks like it's trying to cover both the end consumers (uart uses
>>> uart3_clk) and the internal clock tree consumers (a crystal oscillator
>>> connects to a PLL or a mux has multiple parents). We can certainly use
>>> these bindings for muxes and internal parent-child relationships but I
>>> would prefer we use different bindings for consumer bindings that match
>>> what regulators do today.
>>
>> The binding supports either defining every last internal clock or just
>> the leaf clocks. I took the former route on highbank since I don't have
>> a lot of clocks. If I was doing imx or omap for example, I'd probably
>> just define all the clock controller outputs.
>>
> 
> If only the leaf nodes are defined in DT, then how is the clock platform
> driver implementer supposed to instantiate the rest of the tree and
> connect it up with the partial list of clocks in DT? So, they have to
> switch back and forth between DT and the .c file which defines the rest
> and make sure the parent<->child names match?
> 
> To me it looks that it might better to decouple the description of the
> clock HW from the mapping of a clock leaf to a consumer device. If we just
> use a string to identify the clock that's consumed by a device, we can
> achieve this decoupling at a clean boundary -- clock consumers devices
> (UART) vs clock producer devices (clock controller in the SoC, in a PMIC,
> audio codec, etc).
> 
> With the decoupling, we don't have the inconsistency of having some of the
> clocks of a clock producer device incompletely defined in DT and the rest
> of the clocks of the same clock producer device hard coded in the kernel.
> So, you either put your entire clock tree in the SoC in the DT or put all
> of it in the kernel but you aren't forced to put just some of them in the
> DT just to get DT working. I see no benefit in defining only some of the
> clocks in DT -- it just adds more confusion in the clock tree definition.
> What am I missing?

I fail to see what would need changing in the binding itself. The
binding just describes connections. Whether that is a connection to a
clock controller node to a device or a clock gate/mux/divider node to a
device is really beyond the clock binding. This is really just policy.
You are free to put no clocks in DT, all clocks, or a nexus of clocks.

Rob

Re: [GIT PULL] DT clk binding support

[Saravana Kannan]

On 05/23/2012 06:59 AM, Rob Herring wrote:
> On 05/22/2012 08:38 PM, Saravana Kannan wrote:
>>
>> On Tue, May 22, 2012 6:52 am, Rob Herring wrote:
>>> On 05/21/2012 11:17 PM, Stephen Boyd wrote:
>>>> On 05/21/12 19:15, Shawn Guo wrote:
>>>>> On Mon, May 21, 2012 at 06:52:37PM -0500, Rob Herring wrote:
>>>>>> As Grant states: "This proposed binding is only about one thing:
>>>>>> attaching clock providers to clock consumers." This means you have to
>>>>>> have at least a single provider and a single consumer defined in the
>>>>>> DT.
>>>>>>
>>>>> I just read through Grant's comments over again.  I agree with the
>>>>> statement which implicitly requires the clk provider defined in DT.
>>>>> However, for some case, this provider in DT is just a skeleton which
>>>>> is backed by clock driver where the provider is actually defined.
>>>>>
>>>>> Looking at Grant's comment below, the second option is also to match
>>>>> the clock in driver just using name.  The only difference to my
>>>>> proposal is the name here is given by the argument of phandle pointing
>>>>> to that skeleton provider node.
>>>>>
>>>>> I'm fine with that.  So go ahead with your bindings.
>>>>>
>>>>
>>>> Can we do what the regulator framework has done and have a common
>>>> binding of<connection_name>-clk =<&phandle>? Something like:
>>>>
>>>>      core-clk =<&uart3_clk>
>>>>
>>>> and then have clk_get() use the of node of the device passed in to find
>>>> a property named %s-clk and find the clock with the matching phandle.
>>>
>>> Sigh... That is what we had in previous versions from over a year ago
>>> and we moved away from that approach. The current binding has been
>>> reviewed multiple times in the last 6 months...
>>>
>>> The current approach is aligned with how interrupts are handled (with
>>> the addition of a phandle). I think not having per clock property names
>>> is easier to parse and easier to document.
>>>
>>>> This looks like it's trying to cover both the end consumers (uart uses
>>>> uart3_clk) and the internal clock tree consumers (a crystal oscillator
>>>> connects to a PLL or a mux has multiple parents). We can certainly use
>>>> these bindings for muxes and internal parent-child relationships but I
>>>> would prefer we use different bindings for consumer bindings that match
>>>> what regulators do today.
>>>
>>> The binding supports either defining every last internal clock or just
>>> the leaf clocks. I took the former route on highbank since I don't have
>>> a lot of clocks. If I was doing imx or omap for example, I'd probably
>>> just define all the clock controller outputs.
>>>
>>
>> If only the leaf nodes are defined in DT, then how is the clock platform
>> driver implementer supposed to instantiate the rest of the tree and
>> connect it up with the partial list of clocks in DT? So, they have to
>> switch back and forth between DT and the .c file which defines the rest
>> and make sure the parent<->child names match?
>>
>> To me it looks that it might better to decouple the description of the
>> clock HW from the mapping of a clock leaf to a consumer device. If we just
>> use a string to identify the clock that's consumed by a device, we can
>> achieve this decoupling at a clean boundary -- clock consumers devices
>> (UART) vs clock producer devices (clock controller in the SoC, in a PMIC,
>> audio codec, etc).
>>
>> With the decoupling, we don't have the inconsistency of having some of the
>> clocks of a clock producer device incompletely defined in DT and the rest
>> of the clocks of the same clock producer device hard coded in the kernel.
>> So, you either put your entire clock tree in the SoC in the DT or put all
>> of it in the kernel but you aren't forced to put just some of them in the
>> DT just to get DT working. I see no benefit in defining only some of the
>> clocks in DT -- it just adds more confusion in the clock tree definition.
>> What am I missing?
>
> I fail to see what would need changing in the binding itself. The
> binding just describes connections. Whether that is a connection to a
> clock controller node to a device or a clock gate/mux/divider node to a
> device is really beyond the clock binding. This is really just policy.
> You are free to put no clocks in DT, all clocks, or a nexus of clocks.

With the current approach you are taking can you please give an example 
of how a random device described in DT would hook itself up with a leaf 
clock if that leaf clock is not described in DT? So that it can do a 
call a DT version of clk_get() to get the clock it cares for.

And no, there is a huge difference between binding a clock controller 
node (by which I mean the block that provides many clocks) to a device 
vs. binding a clock leaf to a device. The former is useless wrt to 
clk_get() and similar functions. The latter is very useful to handle that.

Thanks,
Saravana


-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[GIT PULL] DT clk binding support

[Saravana Kannan]

On 05/23/2012 06:59 AM, Rob Herring wrote:
> On 05/22/2012 08:38 PM, Saravana Kannan wrote:
>>
>> On Tue, May 22, 2012 6:52 am, Rob Herring wrote:
>>> On 05/21/2012 11:17 PM, Stephen Boyd wrote:
>>>> On 05/21/12 19:15, Shawn Guo wrote:
>>>>> On Mon, May 21, 2012 at 06:52:37PM -0500, Rob Herring wrote:
>>>>>> As Grant states: "This proposed binding is only about one thing:
>>>>>> attaching clock providers to clock consumers." This means you have to
>>>>>> have at least a single provider and a single consumer defined in the
>>>>>> DT.
>>>>>>
>>>>> I just read through Grant's comments over again.  I agree with the
>>>>> statement which implicitly requires the clk provider defined in DT.
>>>>> However, for some case, this provider in DT is just a skeleton which
>>>>> is backed by clock driver where the provider is actually defined.
>>>>>
>>>>> Looking at Grant's comment below, the second option is also to match
>>>>> the clock in driver just using name.  The only difference to my
>>>>> proposal is the name here is given by the argument of phandle pointing
>>>>> to that skeleton provider node.
>>>>>
>>>>> I'm fine with that.  So go ahead with your bindings.
>>>>>
>>>>
>>>> Can we do what the regulator framework has done and have a common
>>>> binding of<connection_name>-clk =<&phandle>? Something like:
>>>>
>>>>      core-clk =<&uart3_clk>
>>>>
>>>> and then have clk_get() use the of node of the device passed in to find
>>>> a property named %s-clk and find the clock with the matching phandle.
>>>
>>> Sigh... That is what we had in previous versions from over a year ago
>>> and we moved away from that approach. The current binding has been
>>> reviewed multiple times in the last 6 months...
>>>
>>> The current approach is aligned with how interrupts are handled (with
>>> the addition of a phandle). I think not having per clock property names
>>> is easier to parse and easier to document.
>>>
>>>> This looks like it's trying to cover both the end consumers (uart uses
>>>> uart3_clk) and the internal clock tree consumers (a crystal oscillator
>>>> connects to a PLL or a mux has multiple parents). We can certainly use
>>>> these bindings for muxes and internal parent-child relationships but I
>>>> would prefer we use different bindings for consumer bindings that match
>>>> what regulators do today.
>>>
>>> The binding supports either defining every last internal clock or just
>>> the leaf clocks. I took the former route on highbank since I don't have
>>> a lot of clocks. If I was doing imx or omap for example, I'd probably
>>> just define all the clock controller outputs.
>>>
>>
>> If only the leaf nodes are defined in DT, then how is the clock platform
>> driver implementer supposed to instantiate the rest of the tree and
>> connect it up with the partial list of clocks in DT? So, they have to
>> switch back and forth between DT and the .c file which defines the rest
>> and make sure the parent<->child names match?
>>
>> To me it looks that it might better to decouple the description of the
>> clock HW from the mapping of a clock leaf to a consumer device. If we just
>> use a string to identify the clock that's consumed by a device, we can
>> achieve this decoupling at a clean boundary -- clock consumers devices
>> (UART) vs clock producer devices (clock controller in the SoC, in a PMIC,
>> audio codec, etc).
>>
>> With the decoupling, we don't have the inconsistency of having some of the
>> clocks of a clock producer device incompletely defined in DT and the rest
>> of the clocks of the same clock producer device hard coded in the kernel.
>> So, you either put your entire clock tree in the SoC in the DT or put all
>> of it in the kernel but you aren't forced to put just some of them in the
>> DT just to get DT working. I see no benefit in defining only some of the
>> clocks in DT -- it just adds more confusion in the clock tree definition.
>> What am I missing?
>
> I fail to see what would need changing in the binding itself. The
> binding just describes connections. Whether that is a connection to a
> clock controller node to a device or a clock gate/mux/divider node to a
> device is really beyond the clock binding. This is really just policy.
> You are free to put no clocks in DT, all clocks, or a nexus of clocks.

With the current approach you are taking can you please give an example 
of how a random device described in DT would hook itself up with a leaf 
clock if that leaf clock is not described in DT? So that it can do a 
call a DT version of clk_get() to get the clock it cares for.

And no, there is a huge difference between binding a clock controller 
node (by which I mean the block that provides many clocks) to a device 
vs. binding a clock leaf to a device. The former is useless wrt to 
clk_get() and similar functions. The latter is very useful to handle that.

Thanks,
Saravana


-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [GIT PULL] DT clk binding support

[Rob Herring]

On 05/24/2012 04:16 PM, Saravana Kannan wrote:
> On 05/23/2012 06:59 AM, Rob Herring wrote:
>> On 05/22/2012 08:38 PM, Saravana Kannan wrote:

snip

>>> If only the leaf nodes are defined in DT, then how is the clock platform
>>> driver implementer supposed to instantiate the rest of the tree and
>>> connect it up with the partial list of clocks in DT? So, they have to
>>> switch back and forth between DT and the .c file which defines the rest
>>> and make sure the parent<->child names match?
>>>
>>> To me it looks that it might better to decouple the description of the
>>> clock HW from the mapping of a clock leaf to a consumer device. If we
>>> just
>>> use a string to identify the clock that's consumed by a device, we can
>>> achieve this decoupling at a clean boundary -- clock consumers devices
>>> (UART) vs clock producer devices (clock controller in the SoC, in a
>>> PMIC,
>>> audio codec, etc).
>>>
>>> With the decoupling, we don't have the inconsistency of having some
>>> of the
>>> clocks of a clock producer device incompletely defined in DT and the
>>> rest
>>> of the clocks of the same clock producer device hard coded in the
>>> kernel.
>>> So, you either put your entire clock tree in the SoC in the DT or put
>>> all
>>> of it in the kernel but you aren't forced to put just some of them in
>>> the
>>> DT just to get DT working. I see no benefit in defining only some of the
>>> clocks in DT -- it just adds more confusion in the clock tree
>>> definition.
>>> What am I missing?
>>
>> I fail to see what would need changing in the binding itself. The
>> binding just describes connections. Whether that is a connection to a
>> clock controller node to a device or a clock gate/mux/divider node to a
>> device is really beyond the clock binding. This is really just policy.
>> You are free to put no clocks in DT, all clocks, or a nexus of clocks.
> 
> With the current approach you are taking can you please give an example
> of how a random device described in DT would hook itself up with a leaf
> clock if that leaf clock is not described in DT? So that it can do a
> call a DT version of clk_get() to get the clock it cares for.

No, because that's impossible with any binding. The only way that would
work is provide a string with a clock name and matching to the struct
clk name string. That means putting linux clock names into the h/w
description. That is the wrong direction and not how bindings work.
Defining bindings should not get tangled up with how the OS
implementation is done.

> And no, there is a huge difference between binding a clock controller
> node (by which I mean the block that provides many clocks) to a device
> vs. binding a clock leaf to a device. The former is useless wrt to
> clk_get() and similar functions. The latter is very useful to handle that.

The binding and clkdev changes support clk_get fully. Drivers don't have
to change at all. There is not a DT version of clk_get that all drivers
have to adopt. It's all handled within clk_get and should be transparent
to drivers. The only thing that has to change is callers of clk_get_sys
to use of_clk_get, but that's a small fraction of clocks.

Rob

[GIT PULL] DT clk binding support

[Rob Herring]

On 05/24/2012 04:16 PM, Saravana Kannan wrote:
> On 05/23/2012 06:59 AM, Rob Herring wrote:
>> On 05/22/2012 08:38 PM, Saravana Kannan wrote:

snip

>>> If only the leaf nodes are defined in DT, then how is the clock platform
>>> driver implementer supposed to instantiate the rest of the tree and
>>> connect it up with the partial list of clocks in DT? So, they have to
>>> switch back and forth between DT and the .c file which defines the rest
>>> and make sure the parent<->child names match?
>>>
>>> To me it looks that it might better to decouple the description of the
>>> clock HW from the mapping of a clock leaf to a consumer device. If we
>>> just
>>> use a string to identify the clock that's consumed by a device, we can
>>> achieve this decoupling at a clean boundary -- clock consumers devices
>>> (UART) vs clock producer devices (clock controller in the SoC, in a
>>> PMIC,
>>> audio codec, etc).
>>>
>>> With the decoupling, we don't have the inconsistency of having some
>>> of the
>>> clocks of a clock producer device incompletely defined in DT and the
>>> rest
>>> of the clocks of the same clock producer device hard coded in the
>>> kernel.
>>> So, you either put your entire clock tree in the SoC in the DT or put
>>> all
>>> of it in the kernel but you aren't forced to put just some of them in
>>> the
>>> DT just to get DT working. I see no benefit in defining only some of the
>>> clocks in DT -- it just adds more confusion in the clock tree
>>> definition.
>>> What am I missing?
>>
>> I fail to see what would need changing in the binding itself. The
>> binding just describes connections. Whether that is a connection to a
>> clock controller node to a device or a clock gate/mux/divider node to a
>> device is really beyond the clock binding. This is really just policy.
>> You are free to put no clocks in DT, all clocks, or a nexus of clocks.
> 
> With the current approach you are taking can you please give an example
> of how a random device described in DT would hook itself up with a leaf
> clock if that leaf clock is not described in DT? So that it can do a
> call a DT version of clk_get() to get the clock it cares for.

No, because that's impossible with any binding. The only way that would
work is provide a string with a clock name and matching to the struct
clk name string. That means putting linux clock names into the h/w
description. That is the wrong direction and not how bindings work.
Defining bindings should not get tangled up with how the OS
implementation is done.

> And no, there is a huge difference between binding a clock controller
> node (by which I mean the block that provides many clocks) to a device
> vs. binding a clock leaf to a device. The former is useless wrt to
> clk_get() and similar functions. The latter is very useful to handle that.

The binding and clkdev changes support clk_get fully. Drivers don't have
to change at all. There is not a DT version of clk_get that all drivers
have to adopt. It's all handled within clk_get and should be transparent
to drivers. The only thing that has to change is callers of clk_get_sys
to use of_clk_get, but that's a small fraction of clocks.

Rob

Re: [GIT PULL] DT clk binding support

[Saravana Kannan]

On 05/24/2012 02:54 PM, Rob Herring wrote:
> On 05/24/2012 04:16 PM, Saravana Kannan wrote:
>> On 05/23/2012 06:59 AM, Rob Herring wrote:
>>> On 05/22/2012 08:38 PM, Saravana Kannan wrote:
>
> snip
>
>>>> If only the leaf nodes are defined in DT, then how is the clock platform
>>>> driver implementer supposed to instantiate the rest of the tree and
>>>> connect it up with the partial list of clocks in DT? So, they have to
>>>> switch back and forth between DT and the .c file which defines the rest
>>>> and make sure the parent<->child names match?
>>>>
>>>> To me it looks that it might better to decouple the description of the
>>>> clock HW from the mapping of a clock leaf to a consumer device. If we
>>>> just
>>>> use a string to identify the clock that's consumed by a device, we can
>>>> achieve this decoupling at a clean boundary -- clock consumers devices
>>>> (UART) vs clock producer devices (clock controller in the SoC, in a
>>>> PMIC,
>>>> audio codec, etc).
>>>>
>>>> With the decoupling, we don't have the inconsistency of having some
>>>> of the
>>>> clocks of a clock producer device incompletely defined in DT and the
>>>> rest
>>>> of the clocks of the same clock producer device hard coded in the
>>>> kernel.
>>>> So, you either put your entire clock tree in the SoC in the DT or put
>>>> all
>>>> of it in the kernel but you aren't forced to put just some of them in
>>>> the
>>>> DT just to get DT working. I see no benefit in defining only some of the
>>>> clocks in DT -- it just adds more confusion in the clock tree
>>>> definition.
>>>> What am I missing?
>>>
>>> I fail to see what would need changing in the binding itself. The
>>> binding just describes connections. Whether that is a connection to a
>>> clock controller node to a device or a clock gate/mux/divider node to a
>>> device is really beyond the clock binding. This is really just policy.
>>> You are free to put no clocks in DT, all clocks, or a nexus of clocks.
>>
>> With the current approach you are taking can you please give an example
>> of how a random device described in DT would hook itself up with a leaf
>> clock if that leaf clock is not described in DT? So that it can do a
>> call a DT version of clk_get() to get the clock it cares for.
>
> No, because that's impossible with any binding.

So, this is really forcing everyone moving rest of their devices to DT 
to put part/all of the clocks in DT. Either that or deal with the "aux 
data" that's supposed to be temporary.

> The only way that would
> work is provide a string with a clock name and matching to the struct
> clk name string. That means putting linux clock names into the h/w
> description.

No, the name of the clocks stored in clk->name should be the name of the 
clock in HW.

> That is the wrong direction and not how bindings work.
> Defining bindings should not get tangled up with how the OS
> implementation is done.

So, I'm not asking to bind to the name of a clock as defined by the OS 
implementation. I'm just asking to bind to the name of the clock in HW 
instead of the name binding to an actual DT clock node. And I'm only 
asking this because we seem to want to give an option to NOT have the 
clocks in DT.

A developer might choose to abbreviate the clock name in DT and in 
clk->name field in the kernel, but there's nothing wrong with it. Nor 
can DT do anything about it for ANY (not just clock) DT device descriptions.

>> And no, there is a huge difference between binding a clock controller
>> node (by which I mean the block that provides many clocks) to a device
>> vs. binding a clock leaf to a device. The former is useless wrt to
>> clk_get() and similar functions. The latter is very useful to handle that.
>
> The binding and clkdev changes support clk_get fully. Drivers don't have
> to change at all. There is not a DT version of clk_get that all drivers
> have to adopt. It's all handled within clk_get and should be transparent
> to drivers. The only thing that has to change is callers of clk_get_sys
> to use of_clk_get, but that's a small fraction of clocks.

I understand there are changes to have clk_get() work on clocks added 
through DT. But, you said earlier that it doesn't matter if a general DT 
device binds to a clock controller DT device or to an actual clock node 
(say, a leaf in the clock tree) defined in DT. I'm just pointing out 
that it's not true. If a general DT device binds to a clock controller 
DT device, there is no way to figure out what clock node/leaf from the 
clock controller is actually consumed by this general device.

-Saravana

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[GIT PULL] DT clk binding support

[Saravana Kannan]

On 05/24/2012 02:54 PM, Rob Herring wrote:
> On 05/24/2012 04:16 PM, Saravana Kannan wrote:
>> On 05/23/2012 06:59 AM, Rob Herring wrote:
>>> On 05/22/2012 08:38 PM, Saravana Kannan wrote:
>
> snip
>
>>>> If only the leaf nodes are defined in DT, then how is the clock platform
>>>> driver implementer supposed to instantiate the rest of the tree and
>>>> connect it up with the partial list of clocks in DT? So, they have to
>>>> switch back and forth between DT and the .c file which defines the rest
>>>> and make sure the parent<->child names match?
>>>>
>>>> To me it looks that it might better to decouple the description of the
>>>> clock HW from the mapping of a clock leaf to a consumer device. If we
>>>> just
>>>> use a string to identify the clock that's consumed by a device, we can
>>>> achieve this decoupling at a clean boundary -- clock consumers devices
>>>> (UART) vs clock producer devices (clock controller in the SoC, in a
>>>> PMIC,
>>>> audio codec, etc).
>>>>
>>>> With the decoupling, we don't have the inconsistency of having some
>>>> of the
>>>> clocks of a clock producer device incompletely defined in DT and the
>>>> rest
>>>> of the clocks of the same clock producer device hard coded in the
>>>> kernel.
>>>> So, you either put your entire clock tree in the SoC in the DT or put
>>>> all
>>>> of it in the kernel but you aren't forced to put just some of them in
>>>> the
>>>> DT just to get DT working. I see no benefit in defining only some of the
>>>> clocks in DT -- it just adds more confusion in the clock tree
>>>> definition.
>>>> What am I missing?
>>>
>>> I fail to see what would need changing in the binding itself. The
>>> binding just describes connections. Whether that is a connection to a
>>> clock controller node to a device or a clock gate/mux/divider node to a
>>> device is really beyond the clock binding. This is really just policy.
>>> You are free to put no clocks in DT, all clocks, or a nexus of clocks.
>>
>> With the current approach you are taking can you please give an example
>> of how a random device described in DT would hook itself up with a leaf
>> clock if that leaf clock is not described in DT? So that it can do a
>> call a DT version of clk_get() to get the clock it cares for.
>
> No, because that's impossible with any binding.

So, this is really forcing everyone moving rest of their devices to DT 
to put part/all of the clocks in DT. Either that or deal with the "aux 
data" that's supposed to be temporary.

> The only way that would
> work is provide a string with a clock name and matching to the struct
> clk name string. That means putting linux clock names into the h/w
> description.

No, the name of the clocks stored in clk->name should be the name of the 
clock in HW.

> That is the wrong direction and not how bindings work.
> Defining bindings should not get tangled up with how the OS
> implementation is done.

So, I'm not asking to bind to the name of a clock as defined by the OS 
implementation. I'm just asking to bind to the name of the clock in HW 
instead of the name binding to an actual DT clock node. And I'm only 
asking this because we seem to want to give an option to NOT have the 
clocks in DT.

A developer might choose to abbreviate the clock name in DT and in 
clk->name field in the kernel, but there's nothing wrong with it. Nor 
can DT do anything about it for ANY (not just clock) DT device descriptions.

>> And no, there is a huge difference between binding a clock controller
>> node (by which I mean the block that provides many clocks) to a device
>> vs. binding a clock leaf to a device. The former is useless wrt to
>> clk_get() and similar functions. The latter is very useful to handle that.
>
> The binding and clkdev changes support clk_get fully. Drivers don't have
> to change at all. There is not a DT version of clk_get that all drivers
> have to adopt. It's all handled within clk_get and should be transparent
> to drivers. The only thing that has to change is callers of clk_get_sys
> to use of_clk_get, but that's a small fraction of clocks.

I understand there are changes to have clk_get() work on clocks added 
through DT. But, you said earlier that it doesn't matter if a general DT 
device binds to a clock controller DT device or to an actual clock node 
(say, a leaf in the clock tree) defined in DT. I'm just pointing out 
that it's not true. If a general DT device binds to a clock controller 
DT device, there is no way to figure out what clock node/leaf from the 
clock controller is actually consumed by this general device.

-Saravana

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [GIT PULL] DT clk binding support

[Rob Herring]

On 05/24/2012 10:33 PM, Saravana Kannan wrote:
> On 05/24/2012 02:54 PM, Rob Herring wrote:
>> On 05/24/2012 04:16 PM, Saravana Kannan wrote:
>>> On 05/23/2012 06:59 AM, Rob Herring wrote:
>>>> On 05/22/2012 08:38 PM, Saravana Kannan wrote:
>>
>> snip
>>
>>>>> If only the leaf nodes are defined in DT, then how is the clock
>>>>> platform
>>>>> driver implementer supposed to instantiate the rest of the tree and
>>>>> connect it up with the partial list of clocks in DT? So, they have to
>>>>> switch back and forth between DT and the .c file which defines the
>>>>> rest
>>>>> and make sure the parent<->child names match?
>>>>>
>>>>> To me it looks that it might better to decouple the description of the
>>>>> clock HW from the mapping of a clock leaf to a consumer device. If we
>>>>> just
>>>>> use a string to identify the clock that's consumed by a device, we can
>>>>> achieve this decoupling at a clean boundary -- clock consumers devices
>>>>> (UART) vs clock producer devices (clock controller in the SoC, in a
>>>>> PMIC,
>>>>> audio codec, etc).
>>>>>
>>>>> With the decoupling, we don't have the inconsistency of having some
>>>>> of the
>>>>> clocks of a clock producer device incompletely defined in DT and the
>>>>> rest
>>>>> of the clocks of the same clock producer device hard coded in the
>>>>> kernel.
>>>>> So, you either put your entire clock tree in the SoC in the DT or put
>>>>> all
>>>>> of it in the kernel but you aren't forced to put just some of them in
>>>>> the
>>>>> DT just to get DT working. I see no benefit in defining only some
>>>>> of the
>>>>> clocks in DT -- it just adds more confusion in the clock tree
>>>>> definition.
>>>>> What am I missing?
>>>>
>>>> I fail to see what would need changing in the binding itself. The
>>>> binding just describes connections. Whether that is a connection to a
>>>> clock controller node to a device or a clock gate/mux/divider node to a
>>>> device is really beyond the clock binding. This is really just policy.
>>>> You are free to put no clocks in DT, all clocks, or a nexus of clocks.
>>>
>>> With the current approach you are taking can you please give an example
>>> of how a random device described in DT would hook itself up with a leaf
>>> clock if that leaf clock is not described in DT? So that it can do a
>>> call a DT version of clk_get() to get the clock it cares for.
>>
>> No, because that's impossible with any binding.
> 
> So, this is really forcing everyone moving rest of their devices to DT
> to put part/all of the clocks in DT. Either that or deal with the "aux
> data" that's supposed to be temporary.
> 

No. You can still use clkdev lookups if clock lookup is the only thing
you need to hook up. In other words, I would only use aux_data if you
need to set platform_data. If you only need clk lookups, then you can
use clkdev. This is what most of the DT conversions I've reviewed have done.

>> The only way that would
>> work is provide a string with a clock name and matching to the struct
>> clk name string. That means putting linux clock names into the h/w
>> description.
> 
> No, the name of the clocks stored in clk->name should be the name of the
> clock in HW.
> 

True, but it had been debated whether a name was needed for anything but
debug. We're trying to get data like this out of the kernel with DT. For
example, say you have a new derivative SOC that adds 1 additional PLL.
Adding support for that PLL could be as simple as adding it to DT and
the kernel will instantiate it. If you are matching with strings, then
you have to add the new PLL name string to the kernel.

>> That is the wrong direction and not how bindings work.
>> Defining bindings should not get tangled up with how the OS
>> implementation is done.
> 
> So, I'm not asking to bind to the name of a clock as defined by the OS
> implementation. I'm just asking to bind to the name of the clock in HW
> instead of the name binding to an actual DT clock node. And I'm only
> asking this because we seem to want to give an option to NOT have the
> clocks in DT.
> 
> A developer might choose to abbreviate the clock name in DT and in
> clk->name field in the kernel, but there's nothing wrong with it. Nor
> can DT do anything about it for ANY (not just clock) DT device
> descriptions.
> 
>>> And no, there is a huge difference between binding a clock controller
>>> node (by which I mean the block that provides many clocks) to a device
>>> vs. binding a clock leaf to a device. The former is useless wrt to
>>> clk_get() and similar functions. The latter is very useful to handle
>>> that.
>>
>> The binding and clkdev changes support clk_get fully. Drivers don't have
>> to change at all. There is not a DT version of clk_get that all drivers
>> have to adopt. It's all handled within clk_get and should be transparent
>> to drivers. The only thing that has to change is callers of clk_get_sys
>> to use of_clk_get, but that's a small fraction of clocks.
> 
> I understand there are changes to have clk_get() work on clocks added
> through DT. But, you said earlier that it doesn't matter if a general DT
> device binds to a clock controller DT device or to an actual clock node
> (say, a leaf in the clock tree) defined in DT. I'm just pointing out
> that it's not true. If a general DT device binds to a clock controller
> DT device, there is no way to figure out what clock node/leaf from the
> clock controller is actually consumed by this general device.

of_clk_get_parent_name (or some variation that gives you parent phandle
too) doesn't do what you want?

The fact that a driver needs to know details about possible parents is a
problem independent of DT. We do need a better way to abstract that.

Rob

> 
> -Saravana
> 

[GIT PULL] DT clk binding support

[Rob Herring]

On 05/24/2012 10:33 PM, Saravana Kannan wrote:
> On 05/24/2012 02:54 PM, Rob Herring wrote:
>> On 05/24/2012 04:16 PM, Saravana Kannan wrote:
>>> On 05/23/2012 06:59 AM, Rob Herring wrote:
>>>> On 05/22/2012 08:38 PM, Saravana Kannan wrote:
>>
>> snip
>>
>>>>> If only the leaf nodes are defined in DT, then how is the clock
>>>>> platform
>>>>> driver implementer supposed to instantiate the rest of the tree and
>>>>> connect it up with the partial list of clocks in DT? So, they have to
>>>>> switch back and forth between DT and the .c file which defines the
>>>>> rest
>>>>> and make sure the parent<->child names match?
>>>>>
>>>>> To me it looks that it might better to decouple the description of the
>>>>> clock HW from the mapping of a clock leaf to a consumer device. If we
>>>>> just
>>>>> use a string to identify the clock that's consumed by a device, we can
>>>>> achieve this decoupling at a clean boundary -- clock consumers devices
>>>>> (UART) vs clock producer devices (clock controller in the SoC, in a
>>>>> PMIC,
>>>>> audio codec, etc).
>>>>>
>>>>> With the decoupling, we don't have the inconsistency of having some
>>>>> of the
>>>>> clocks of a clock producer device incompletely defined in DT and the
>>>>> rest
>>>>> of the clocks of the same clock producer device hard coded in the
>>>>> kernel.
>>>>> So, you either put your entire clock tree in the SoC in the DT or put
>>>>> all
>>>>> of it in the kernel but you aren't forced to put just some of them in
>>>>> the
>>>>> DT just to get DT working. I see no benefit in defining only some
>>>>> of the
>>>>> clocks in DT -- it just adds more confusion in the clock tree
>>>>> definition.
>>>>> What am I missing?
>>>>
>>>> I fail to see what would need changing in the binding itself. The
>>>> binding just describes connections. Whether that is a connection to a
>>>> clock controller node to a device or a clock gate/mux/divider node to a
>>>> device is really beyond the clock binding. This is really just policy.
>>>> You are free to put no clocks in DT, all clocks, or a nexus of clocks.
>>>
>>> With the current approach you are taking can you please give an example
>>> of how a random device described in DT would hook itself up with a leaf
>>> clock if that leaf clock is not described in DT? So that it can do a
>>> call a DT version of clk_get() to get the clock it cares for.
>>
>> No, because that's impossible with any binding.
> 
> So, this is really forcing everyone moving rest of their devices to DT
> to put part/all of the clocks in DT. Either that or deal with the "aux
> data" that's supposed to be temporary.
> 

No. You can still use clkdev lookups if clock lookup is the only thing
you need to hook up. In other words, I would only use aux_data if you
need to set platform_data. If you only need clk lookups, then you can
use clkdev. This is what most of the DT conversions I've reviewed have done.

>> The only way that would
>> work is provide a string with a clock name and matching to the struct
>> clk name string. That means putting linux clock names into the h/w
>> description.
> 
> No, the name of the clocks stored in clk->name should be the name of the
> clock in HW.
> 

True, but it had been debated whether a name was needed for anything but
debug. We're trying to get data like this out of the kernel with DT. For
example, say you have a new derivative SOC that adds 1 additional PLL.
Adding support for that PLL could be as simple as adding it to DT and
the kernel will instantiate it. If you are matching with strings, then
you have to add the new PLL name string to the kernel.

>> That is the wrong direction and not how bindings work.
>> Defining bindings should not get tangled up with how the OS
>> implementation is done.
> 
> So, I'm not asking to bind to the name of a clock as defined by the OS
> implementation. I'm just asking to bind to the name of the clock in HW
> instead of the name binding to an actual DT clock node. And I'm only
> asking this because we seem to want to give an option to NOT have the
> clocks in DT.
> 
> A developer might choose to abbreviate the clock name in DT and in
> clk->name field in the kernel, but there's nothing wrong with it. Nor
> can DT do anything about it for ANY (not just clock) DT device
> descriptions.
> 
>>> And no, there is a huge difference between binding a clock controller
>>> node (by which I mean the block that provides many clocks) to a device
>>> vs. binding a clock leaf to a device. The former is useless wrt to
>>> clk_get() and similar functions. The latter is very useful to handle
>>> that.
>>
>> The binding and clkdev changes support clk_get fully. Drivers don't have
>> to change at all. There is not a DT version of clk_get that all drivers
>> have to adopt. It's all handled within clk_get and should be transparent
>> to drivers. The only thing that has to change is callers of clk_get_sys
>> to use of_clk_get, but that's a small fraction of clocks.
> 
> I understand there are changes to have clk_get() work on clocks added
> through DT. But, you said earlier that it doesn't matter if a general DT
> device binds to a clock controller DT device or to an actual clock node
> (say, a leaf in the clock tree) defined in DT. I'm just pointing out
> that it's not true. If a general DT device binds to a clock controller
> DT device, there is no way to figure out what clock node/leaf from the
> clock controller is actually consumed by this general device.

of_clk_get_parent_name (or some variation that gives you parent phandle
too) doesn't do what you want?

The fact that a driver needs to know details about possible parents is a
problem independent of DT. We do need a better way to abstract that.

Rob

> 
> -Saravana
>