diff mbox

[GIT,PULL] Amlogic 64-bit DT updates for v4.12

Message ID CAK8P3a02e3DxjuW9pA66NSCc9vw_h1Xuk6TdEbRnEQ4v4W7NHA@mail.gmail.com (mailing list archive)
State Not Applicable
Delegated to: Stephen Boyd
Headers show

Commit Message

Arnd Bergmann March 31, 2017, 2:02 p.m. UTC
On Wed, Mar 29, 2017 at 11:45 PM, Kevin Hilman <khilman@baylibre.com> wrote:
> Olof, Arnd,
>
> Below are some 64-bit DT changes for Amlogic platforms for v4.12.  Note
> that this branch includes a merge of an immutable branch from the clk
> tree (clk-meson) due to dependencies on new clocks added to the meson
> clock driver and used in these DT updates.

I did not pull this one because of the silly dependency. The device tree
files should be completely independent of the driver changes.

Here I see commits like these:

commit fd86d3edb417d5c719fa0ffc4adf9b99e1f2db48
Author: Neil Armstrong <narmstrong@baylibre.com>
Date:   Wed Mar 22 11:32:26 2017 +0100

    clk: meson-gxbb: Expose GP0 dt-bindings clock id

    This patch exposes the GP0 PLL clock id in the dt bindings.

    Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
    Signed-off-by: Michael Turquette <mturquette@baylibre.com>
    Link: lkml.kernel.org/r/1490178747-14837-5-git-send-email-narmstrong@baylibre.com


Which add random macros in the middle of the number space.

Since the header changes are in an immutable branch, it's probably
too late to fix this for 4.12, but maybe we can get the header into a
form that allows us to do future DT changes without having to
wait an extra release. The clock driver apparently already defines
some of the numbers above, so you can fill all of those in already.

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

Comments

Kevin Hilman March 31, 2017, 4:01 p.m. UTC | #1
Arnd Bergmann <arnd@arndb.de> writes:

> On Wed, Mar 29, 2017 at 11:45 PM, Kevin Hilman <khilman@baylibre.com> wrote:
>> Olof, Arnd,
>>
>> Below are some 64-bit DT changes for Amlogic platforms for v4.12.  Note
>> that this branch includes a merge of an immutable branch from the clk
>> tree (clk-meson) due to dependencies on new clocks added to the meson
>> clock driver and used in these DT updates.
>
> I did not pull this one because of the silly dependency. The device tree
> files should be completely independent of the driver changes.

Aaargh, this is getting very frustrating.

[...]

> Which add random macros in the middle of the number space.
>
> Since the header changes are in an immutable branch, it's probably
> too late to fix this for 4.12, but maybe we can get the header into a
> form that allows us to do future DT changes without having to
> wait an extra release. The clock driver apparently already defines
> some of the numbers above, so you can fill all of those in already.

We could, but this is in direct conflict with the recommendations of the
clk maintainers to not expose clock IDs to the DT until we know they are
used because, well, things change.

Can we *please* get some clarity on on this between arm-soc and the clk
maintainers?  I've been asking for a couple merge windows now.

Irritated,

Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ben Dooks March 31, 2017, 4:11 p.m. UTC | #2
On 31/03/17 17:01, Kevin Hilman wrote:
> Arnd Bergmann <arnd@arndb.de> writes:
>
>> On Wed, Mar 29, 2017 at 11:45 PM, Kevin Hilman <khilman@baylibre.com> wrote:
>>> Olof, Arnd,
>>>
>>> Below are some 64-bit DT changes for Amlogic platforms for v4.12.  Note
>>> that this branch includes a merge of an immutable branch from the clk
>>> tree (clk-meson) due to dependencies on new clocks added to the meson
>>> clock driver and used in these DT updates.
>>
>> I did not pull this one because of the silly dependency. The device tree
>> files should be completely independent of the driver changes.
>
> Aaargh, this is getting very frustrating.

I tried to make this point very early on in the development,
we don't want to be changing both driver and dt if we are
adding things like IDs for gated clocks.

For example, having an almost 1:1 mapping from clock-id to
bit-in-hardware would make life much easier, even if the
driver had a little bit more code to create clocks at the
probe time, it would have meant that DT additions for new
peripherals would have been much easier.

> [...]
>
>> Which add random macros in the middle of the number space.
>>
>> Since the header changes are in an immutable branch, it's probably
>> too late to fix this for 4.12, but maybe we can get the header into a
>> form that allows us to do future DT changes without having to
>> wait an extra release. The clock driver apparently already defines
>> some of the numbers above, so you can fill all of those in already.
>
> We could, but this is in direct conflict with the recommendations of the
> clk maintainers to not expose clock IDs to the DT until we know they are
> used because, well, things change.
>
> Can we *please* get some clarity on on this between arm-soc and the clk
> maintainers?  I've been asking for a couple merge windows now.
>
> Irritated,
>
> Kevin
>
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic
>
Michael Turquette March 31, 2017, 5:07 p.m. UTC | #3
Quoting Ben Dooks (2017-03-31 09:11:17)
> On 31/03/17 17:01, Kevin Hilman wrote:
> > Arnd Bergmann <arnd@arndb.de> writes:
> >
> >> On Wed, Mar 29, 2017 at 11:45 PM, Kevin Hilman <khilman@baylibre.com> wrote:
> >>> Olof, Arnd,
> >>>
> >>> Below are some 64-bit DT changes for Amlogic platforms for v4.12.  Note
> >>> that this branch includes a merge of an immutable branch from the clk
> >>> tree (clk-meson) due to dependencies on new clocks added to the meson
> >>> clock driver and used in these DT updates.
> >>
> >> I did not pull this one because of the silly dependency. The device tree
> >> files should be completely independent of the driver changes.
> >
> > Aaargh, this is getting very frustrating.
> 
> I tried to make this point very early on in the development,
> we don't want to be changing both driver and dt if we are
> adding things like IDs for gated clocks.
> 
> For example, having an almost 1:1 mapping from clock-id to
> bit-in-hardware would make life much easier, even if the
> driver had a little bit more code to create clocks at the
> probe time, it would have meant that DT additions for new
> peripherals would have been much easier.

It would be great if the hardware was designed in such a way to make
that possible. A lot of clock controller hardware simply isn't.

We're talking about defining a reference to a clock control, somewhere
in the middle of the clock tree, that maybe we didn't know existed until
now. This simply is not the same as enumerating physical pins or
something like that.

To be clear, all of the leaf clock (gates) were defined in DT from the
very beginning for the SoC in question. This is perfectly in-line with
Arnd's world-view. These gates are very orderly and correspond directly
to the consumer devices that take them as input.

It's the mess of clocks that make up the "middle" of the tree that cause
these problems. I don't see why we can't agree on a way to update the DT
headers in such cases.

> 
> > [...]
> >
> >> Which add random macros in the middle of the number space.
> >>
> >> Since the header changes are in an immutable branch, it's probably
> >> too late to fix this for 4.12, but maybe we can get the header into a
> >> form that allows us to do future DT changes without having to
> >> wait an extra release. The clock driver apparently already defines
> >> some of the numbers above, so you can fill all of those in already.
> >
> > We could, but this is in direct conflict with the recommendations of the
> > clk maintainers to not expose clock IDs to the DT until we know they are
> > used because, well, things change.
> >
> > Can we *please* get some clarity on on this between arm-soc and the clk
> > maintainers?  I've been asking for a couple merge windows now.

I'm hopeful that that the arm-soc maintainers can see it my way ;-)

I'll explain the thought process behind the gxbb clk IDs in DT again:

1) we don't know about all the clocks ahead of time. New documentation
is always forthcoming, especially in regards to multimedia-focused IP

2) we do know about all of the leaf clocks that devices take as input
and expose them in DT from the very beginning, as we should

3) ideally we should not need to expose any non-leaf clocks in DT. This
would be akin to exposing private data or state machine internals in DT,
when the consumer nodes only care about a phandle to their input

4) ideally consumer devices should reference only the leaf clocks.
Ideally the framework & clk provider driver should be smart enough to Do
The Right Thing.

Items #3 and #4 are not always true, we don't life in an ideal world and
hence, sometimes we add new clock IDs to DT. This should not be grounds
for a patch not making it into the merge window IMO.

Best regards,
Mike

> >
> > Irritated,
> >
> > Kevin
> >
> > _______________________________________________
> > linux-amlogic mailing list
> > linux-amlogic@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-amlogic
> >
> 
> 
> -- 
> Ben Dooks                               http://www.codethink.co.uk/
> Senior Engineer                         Codethink - Providing Genius
> --
> To unsubscribe from this list: send the line "unsubscribe linux-clk" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/include/dt-bindings/clock/gxbb-clkc.h
b/include/dt-bindings/clock/gxbb-clkc.h
index ef7d6b792405..cce6cb5418f1 100644
--- a/include/dt-bindings/clock/gxbb-clkc.h
+++ b/include/dt-bindings/clock/gxbb-clkc.h
@@ -10,6 +10,7 @@ 
 #define CLKID_FCLK_DIV2                4
 #define CLKID_FCLK_DIV3                5
 #define CLKID_FCLK_DIV4                6
+#define CLKID_GP0_PLL          9
 #define CLKID_CLK81            12
 #define CLKID_MPLL2            15
 #define CLKID_SPI              34

commit 7c4aabd0e19491749488b363fa409956fa3857bb
Author: Neil Armstrong <narmstrong@baylibre.com>
Date:   Wed Mar 22 11:18:53 2017 +0100

    clk: meson-gxbb: Add MALI clock IDS

    Add missing MALI clock IDs and expose the muxes and gates in the
dt-bindings.

    Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
    Signed-off-by: Michael Turquette <mturquette@baylibre.com>
    Link: lkml.kernel.org/r/1490177935-9646-2-git-send-email-narmstrong@baylibre.com

diff --git a/include/dt-bindings/clock/gxbb-clkc.h
b/include/dt-bindings/clock/gxbb-clkc.h
index f08f06dd7702..ef7d6b792405 100644
--- a/include/dt-bindings/clock/gxbb-clkc.h
+++ b/include/dt-bindings/clock/gxbb-clkc.h
@@ -35,5 +35,10 @@ 
 #define CLKID_SD_EMMC_C                96
 #define CLKID_SAR_ADC_CLK      97
 #define CLKID_SAR_ADC_SEL      98
+#define CLKID_MALI_0_SEL       100
+#define CLKID_MALI_0           102
+#define CLKID_MALI_1_SEL       103
+#define CLKID_MALI_1           105
+#define CLKID_MALI             106

 #endif /* __GXBB_CLKC_H */

commit 7e148d522982b29c454a31386eb3d27b10d5adbc
Author: Jerome Brunet <jbrunet@baylibre.com>
Date:   Thu Mar 9 11:41:54 2017 +0100

    dt-bindings: clk: gxbb: expose i2s output clock gates

    Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
    Signed-off-by: Michael Turquette <mturquette@baylibre.com>
    Link: lkml.kernel.org/r/20170309104154.28295-10-jbrunet@baylibre.com

diff --git a/include/dt-bindings/clock/gxbb-clkc.h
b/include/dt-bindings/clock/gxbb-clkc.h
index 692846c7941b..f08f06dd7702 100644
--- a/include/dt-bindings/clock/gxbb-clkc.h
+++ b/include/dt-bindings/clock/gxbb-clkc.h
@@ -16,6 +16,10 @@ 
 #define CLKID_I2C              22
 #define CLKID_SAR_ADC          23
 #define CLKID_ETH              36
+#define CLKID_AIU_GLUE         38
+#define CLKID_I2S_OUT          40
+#define CLKID_MIXER_IFACE      44
+#define CLKID_AIU              47
 #define CLKID_USB0             50
 #define CLKID_USB1             51
 #define CLKID_USB              55