Re: [PATCHv2 4/5] clk: samsung: exynos5410: Add fixed rate clocks
Humberto,
On 31.07.2014 23:01, Humberto Naves wrote:
> Hi,
>
> On Thu, Jul 31, 2014 at 1:45 PM, Sylwester Nawrocki
> wrote:
>> Can you explain what is rationale behind this change ? Is it related to
>> suspend/resume ordering ?
>
> I had forgotten, but now remember the reason why I did this. If you
> see the current implementation of clk-exynos5410, you will notice it
> heavily depends on the clock "fin_pll". On the other hand, this clock
> exists because in the current dtb (exynos5410-smdk5410.dts), there is
> a node fin_pll such as
>
>fin_pll: xxti {
>compatible = "fixed-clock";
>clock-frequency = <2400>;
>clock-output-names = "fin_pll";
>#clock-cells = <0>;
>};
>
> So far so good. But the real problem comes in when I check the rate of
> fin_pll to determine if I should install the rate table or not (and I
> really need this for my patch). More specifically
>
>if (_get_rate("fin_pll") == 24 * MHZ) {
>exynos5410_plls[apll].rate_table = apll_24mhz_tbl;
>exynos5410_plls[cpll].rate_table = cpll_24mhz_tbl;
>exynos5410_plls[kpll].rate_table = kpll_24mhz_tbl;
>exynos5410_plls[dpll].rate_table = dpll_24mhz_tbl;
>exynos5410_plls[epll].rate_table = epll_24mhz_tbl;
>exynos5410_plls[ipll].rate_table = ipll_24mhz_tbl;
>}
>
> I *have* to determine if the rate of fin_pll is 24MHz, and this is
> impossible to do if fin_pll is not available. Moreover, there is no
> way I can ensure that the fixed clock provider for fin_pll was
> initialized before mine, so there is chance that _get_rate won't work.
> The only way I fix that is to set the dependency explicitly in the
> dtb, by adding the fin_pll clock as required resource.
>
> clock: clock-controller@1001 {
> compatible = "samsung,exynos5410-clock";
> reg = <0x1001 0x3>;
> #clock-cells = <1>;
> /* Add the parent clock */
> clocks = <_pll>;
> clock-names = "fin_pll";
> };
This is the correct solution to your problem. The clocks and clock-names
properties should have been there from the beginning but apparently this
has been missed in review. Also see below.
>
> But in any case, the bindings with the DTB must be changed one way or
> another, because I *really* need to use fin_pll on my driver
> registration.
This is a backwards compatible change. On DTBs without clocks and
clock-names properties the PLL tables simply won't be registered which
is exactly the same behavior we have now without any tables in the
driver at all.
> If you agree with this alternative solution I previously
> described, I can change that in the next version of the patch series.
Please update the dts instead, in the way you pointed above.
Best regards,
Tomasz
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2 4/5] clk: samsung: exynos5410: Add fixed rate clocks
Hi,
On Thu, Jul 31, 2014 at 1:45 PM, Sylwester Nawrocki
wrote:
> Can you explain what is rationale behind this change ? Is it related to
> suspend/resume ordering ?
I had forgotten, but now remember the reason why I did this. If you
see the current implementation of clk-exynos5410, you will notice it
heavily depends on the clock "fin_pll". On the other hand, this clock
exists because in the current dtb (exynos5410-smdk5410.dts), there is
a node fin_pll such as
fin_pll: xxti {
compatible = "fixed-clock";
clock-frequency = <2400>;
clock-output-names = "fin_pll";
#clock-cells = <0>;
};
So far so good. But the real problem comes in when I check the rate of
fin_pll to determine if I should install the rate table or not (and I
really need this for my patch). More specifically
if (_get_rate("fin_pll") == 24 * MHZ) {
exynos5410_plls[apll].rate_table = apll_24mhz_tbl;
exynos5410_plls[cpll].rate_table = cpll_24mhz_tbl;
exynos5410_plls[kpll].rate_table = kpll_24mhz_tbl;
exynos5410_plls[dpll].rate_table = dpll_24mhz_tbl;
exynos5410_plls[epll].rate_table = epll_24mhz_tbl;
exynos5410_plls[ipll].rate_table = ipll_24mhz_tbl;
}
I *have* to determine if the rate of fin_pll is 24MHz, and this is
impossible to do if fin_pll is not available. Moreover, there is no
way I can ensure that the fixed clock provider for fin_pll was
initialized before mine, so there is chance that _get_rate won't work.
The only way I fix that is to set the dependency explicitly in the
dtb, by adding the fin_pll clock as required resource.
clock: clock-controller@1001 {
compatible = "samsung,exynos5410-clock";
reg = <0x1001 0x3>;
#clock-cells = <1>;
/* Add the parent clock */
clocks = <_pll>;
clock-names = "fin_pll";
};
But in any case, the bindings with the DTB must be changed one way or
another, because I *really* need to use fin_pll on my driver
registration. If you agree with this alternative solution I previously
described, I can change that in the next version of the patch series.
Best regards,
Humberto
> Obviously it breaks the kernel/dtb compatibility. We should be moving
> in opposite direction, i.e. completely remove the custom samsung fixed
> rate clocks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2 4/5] clk: samsung: exynos5410: Add fixed rate clocks
On 31.07.2014 15:23, Humberto Naves wrote:
> Hi Tomasz,
>
> I perfectly see your point.
> However my question was why you did you decide to postpone
> Sylwester's? Was there any specific reason?
> I suppose it would break all the dtb compatibility, but besides that,
> was there any other reason?
We discussed this in private (we work in the same office) and I pointed
out certain issues with Sylwester's proposed implementation and we
agreed that one more revision of the patch is needed, but as it happens,
higher priority tasks showed up and this one got lost in action.
The first version of the patch [1] changed the original behavior
breaking DT ABI compatibility and relied on improper assumption that
those clocks are always in "fixed-rate-clocks" node. The thing is that
no code should rely on DT node naming.
Second version [2] was much better in this aspect, but it had some minor
implementation issues - custom clk_ops used instead of generic mux clock
and chipid block being constantly mapped and unmapped in every call to
__fin_pll_mux_get_parent().
I should have reviewed them both on the mailing lists, but at that time
there was no major activity related to Exynos4 outside of our office, so
it was more convenient to just talk together directly.
[1] http://www.spinics.net/lists/arm-kernel/msg333211.html
[2]
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-May/258490.html
Anyway, I don't think this is all relevant to Exynos5410, which just
uses the generic fixed rate binding and has the thing done right from
the start.
Best regards,
Tomasz
>
> Best,
> Humberto
>
> On Thu, Jul 31, 2014 at 2:53 PM, Tomasz Figa wrote:
>> Hi Humberto,
>>
>> You can find my comments inline.
>>
>> On 31.07.2014 13:22, Humberto Silva Naves wrote:
>>> This implements the fixed rate clocks generated either inside or
>>> outside the SoC. It also adds a dt-binding constant for the
>>> sclk_hdmiphy clock, which shall be later used by other drivers,
>>> such as the DRM.
>>>
>>> Since the external fixed rate clock fin_pll is now registered by
>>> the clk-exynos5410 file, the bindings with the device tree file have
>>> changed. It is no longer needed to define fin_pll as a fixed clock,
>>> such as in:
>>>
>>> fin_pll: xxti {
>>> compatible = "fixed-clock";
>>> clock-frequency = <2400>;
>>> clock-output-names = "fin_pll";
>>> #clock-cells = <0>;
>>> };
>>>
>>> The above lines should be replaced by the following lines:
>>>
>>> fixed-rate-clocks {
>>> oscclk {
>>> compatible = "samsung,exynos5410-oscclk";
>>> clock-frequency = <2400>;
>>> };
>>> };
>>>
>>> This new form of binding was properly documented in the relevant
>>> documentation file.
>>
>> In general this is backwards. This Exynos-specific clock binding was
>> invented before generic fixed rate clock binding showed up and so few
>> drivers still use it to maintain DT ABI compatibility. However new
>> drivers are required to use the new generic binding and so does the one
>> for Exynos5410.
>>
>> Best regards,
>> Tomasz
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2 4/5] clk: samsung: exynos5410: Add fixed rate clocks
Hi Tomasz,
I perfectly see your point.
However my question was why you did you decide to postpone
Sylwester's? Was there any specific reason?
I suppose it would break all the dtb compatibility, but besides that,
was there any other reason?
Best,
Humberto
On Thu, Jul 31, 2014 at 2:53 PM, Tomasz Figa wrote:
> Hi Humberto,
>
> You can find my comments inline.
>
> On 31.07.2014 13:22, Humberto Silva Naves wrote:
>> This implements the fixed rate clocks generated either inside or
>> outside the SoC. It also adds a dt-binding constant for the
>> sclk_hdmiphy clock, which shall be later used by other drivers,
>> such as the DRM.
>>
>> Since the external fixed rate clock fin_pll is now registered by
>> the clk-exynos5410 file, the bindings with the device tree file have
>> changed. It is no longer needed to define fin_pll as a fixed clock,
>> such as in:
>>
>> fin_pll: xxti {
>> compatible = "fixed-clock";
>> clock-frequency = <2400>;
>> clock-output-names = "fin_pll";
>> #clock-cells = <0>;
>> };
>>
>> The above lines should be replaced by the following lines:
>>
>> fixed-rate-clocks {
>> oscclk {
>> compatible = "samsung,exynos5410-oscclk";
>> clock-frequency = <2400>;
>> };
>> };
>>
>> This new form of binding was properly documented in the relevant
>> documentation file.
>
> In general this is backwards. This Exynos-specific clock binding was
> invented before generic fixed rate clock binding showed up and so few
> drivers still use it to maintain DT ABI compatibility. However new
> drivers are required to use the new generic binding and so does the one
> for Exynos5410.
>
> Best regards,
> Tomasz
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2 4/5] clk: samsung: exynos5410: Add fixed rate clocks
Hi Humberto,
You can find my comments inline.
On 31.07.2014 13:22, Humberto Silva Naves wrote:
> This implements the fixed rate clocks generated either inside or
> outside the SoC. It also adds a dt-binding constant for the
> sclk_hdmiphy clock, which shall be later used by other drivers,
> such as the DRM.
>
> Since the external fixed rate clock fin_pll is now registered by
> the clk-exynos5410 file, the bindings with the device tree file have
> changed. It is no longer needed to define fin_pll as a fixed clock,
> such as in:
>
> fin_pll: xxti {
> compatible = "fixed-clock";
> clock-frequency = <2400>;
> clock-output-names = "fin_pll";
> #clock-cells = <0>;
> };
>
> The above lines should be replaced by the following lines:
>
> fixed-rate-clocks {
> oscclk {
> compatible = "samsung,exynos5410-oscclk";
> clock-frequency = <2400>;
> };
> };
>
> This new form of binding was properly documented in the relevant
> documentation file.
In general this is backwards. This Exynos-specific clock binding was
invented before generic fixed rate clock binding showed up and so few
drivers still use it to maintain DT ABI compatibility. However new
drivers are required to use the new generic binding and so does the one
for Exynos5410.
Best regards,
Tomasz
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2 4/5] clk: samsung: exynos5410: Add fixed rate clocks
(dropping linux-doc ML and Randy from Cc)
On 31/07/14 13:22, Humberto Silva Naves wrote:
> This implements the fixed rate clocks generated either inside or
> outside the SoC. It also adds a dt-binding constant for the
> sclk_hdmiphy clock, which shall be later used by other drivers,
> such as the DRM.
>
> Since the external fixed rate clock fin_pll is now registered by
> the clk-exynos5410 file, the bindings with the device tree file have
> changed. It is no longer needed to define fin_pll as a fixed clock,
> such as in:
>
> fin_pll: xxti {
> compatible = "fixed-clock";
> clock-frequency = <2400>;
> clock-output-names = "fin_pll";
> #clock-cells = <0>;
> };
>
> The above lines should be replaced by the following lines:
>
> fixed-rate-clocks {
> oscclk {
> compatible = "samsung,exynos5410-oscclk";
> clock-frequency = <2400>;
> };
> };
>
> This new form of binding was properly documented in the relevant
> documentation file.
Can you explain what is rationale behind this change ? Is it related to
suspend/resume ordering ?
Obviously it breaks the kernel/dtb compatibility. We should be moving
in opposite direction, i.e. completely remove the custom samsung fixed
rate clocks. I've tried to address this with patches [1], [2] but Tomasz
wasn't happy with them IIRC and I postponed work on that.
[1] http://www.spinics.net/lists/arm-kernel/msg333211.html
[2] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-May/258490.html
--
Regards,
Sylwester
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2 4/5] clk: samsung: exynos5410: Add fixed rate clocks
(dropping linux-doc ML and Randy from Cc)
On 31/07/14 13:22, Humberto Silva Naves wrote:
This implements the fixed rate clocks generated either inside or
outside the SoC. It also adds a dt-binding constant for the
sclk_hdmiphy clock, which shall be later used by other drivers,
such as the DRM.
Since the external fixed rate clock fin_pll is now registered by
the clk-exynos5410 file, the bindings with the device tree file have
changed. It is no longer needed to define fin_pll as a fixed clock,
such as in:
fin_pll: xxti {
compatible = fixed-clock;
clock-frequency = 2400;
clock-output-names = fin_pll;
#clock-cells = 0;
};
The above lines should be replaced by the following lines:
fixed-rate-clocks {
oscclk {
compatible = samsung,exynos5410-oscclk;
clock-frequency = 2400;
};
};
This new form of binding was properly documented in the relevant
documentation file.
Can you explain what is rationale behind this change ? Is it related to
suspend/resume ordering ?
Obviously it breaks the kernel/dtb compatibility. We should be moving
in opposite direction, i.e. completely remove the custom samsung fixed
rate clocks. I've tried to address this with patches [1], [2] but Tomasz
wasn't happy with them IIRC and I postponed work on that.
[1] http://www.spinics.net/lists/arm-kernel/msg333211.html
[2] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-May/258490.html
--
Regards,
Sylwester
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2 4/5] clk: samsung: exynos5410: Add fixed rate clocks
Hi Humberto,
You can find my comments inline.
On 31.07.2014 13:22, Humberto Silva Naves wrote:
This implements the fixed rate clocks generated either inside or
outside the SoC. It also adds a dt-binding constant for the
sclk_hdmiphy clock, which shall be later used by other drivers,
such as the DRM.
Since the external fixed rate clock fin_pll is now registered by
the clk-exynos5410 file, the bindings with the device tree file have
changed. It is no longer needed to define fin_pll as a fixed clock,
such as in:
fin_pll: xxti {
compatible = fixed-clock;
clock-frequency = 2400;
clock-output-names = fin_pll;
#clock-cells = 0;
};
The above lines should be replaced by the following lines:
fixed-rate-clocks {
oscclk {
compatible = samsung,exynos5410-oscclk;
clock-frequency = 2400;
};
};
This new form of binding was properly documented in the relevant
documentation file.
In general this is backwards. This Exynos-specific clock binding was
invented before generic fixed rate clock binding showed up and so few
drivers still use it to maintain DT ABI compatibility. However new
drivers are required to use the new generic binding and so does the one
for Exynos5410.
Best regards,
Tomasz
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2 4/5] clk: samsung: exynos5410: Add fixed rate clocks
Hi Tomasz,
I perfectly see your point.
However my question was why you did you decide to postpone
Sylwester's? Was there any specific reason?
I suppose it would break all the dtb compatibility, but besides that,
was there any other reason?
Best,
Humberto
On Thu, Jul 31, 2014 at 2:53 PM, Tomasz Figa tomasz.f...@gmail.com wrote:
Hi Humberto,
You can find my comments inline.
On 31.07.2014 13:22, Humberto Silva Naves wrote:
This implements the fixed rate clocks generated either inside or
outside the SoC. It also adds a dt-binding constant for the
sclk_hdmiphy clock, which shall be later used by other drivers,
such as the DRM.
Since the external fixed rate clock fin_pll is now registered by
the clk-exynos5410 file, the bindings with the device tree file have
changed. It is no longer needed to define fin_pll as a fixed clock,
such as in:
fin_pll: xxti {
compatible = fixed-clock;
clock-frequency = 2400;
clock-output-names = fin_pll;
#clock-cells = 0;
};
The above lines should be replaced by the following lines:
fixed-rate-clocks {
oscclk {
compatible = samsung,exynos5410-oscclk;
clock-frequency = 2400;
};
};
This new form of binding was properly documented in the relevant
documentation file.
In general this is backwards. This Exynos-specific clock binding was
invented before generic fixed rate clock binding showed up and so few
drivers still use it to maintain DT ABI compatibility. However new
drivers are required to use the new generic binding and so does the one
for Exynos5410.
Best regards,
Tomasz
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2 4/5] clk: samsung: exynos5410: Add fixed rate clocks
On 31.07.2014 15:23, Humberto Naves wrote:
Hi Tomasz,
I perfectly see your point.
However my question was why you did you decide to postpone
Sylwester's? Was there any specific reason?
I suppose it would break all the dtb compatibility, but besides that,
was there any other reason?
We discussed this in private (we work in the same office) and I pointed
out certain issues with Sylwester's proposed implementation and we
agreed that one more revision of the patch is needed, but as it happens,
higher priority tasks showed up and this one got lost in action.
The first version of the patch [1] changed the original behavior
breaking DT ABI compatibility and relied on improper assumption that
those clocks are always in fixed-rate-clocks node. The thing is that
no code should rely on DT node naming.
Second version [2] was much better in this aspect, but it had some minor
implementation issues - custom clk_ops used instead of generic mux clock
and chipid block being constantly mapped and unmapped in every call to
__fin_pll_mux_get_parent().
I should have reviewed them both on the mailing lists, but at that time
there was no major activity related to Exynos4 outside of our office, so
it was more convenient to just talk together directly.
[1] http://www.spinics.net/lists/arm-kernel/msg333211.html
[2]
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-May/258490.html
Anyway, I don't think this is all relevant to Exynos5410, which just
uses the generic fixed rate binding and has the thing done right from
the start.
Best regards,
Tomasz
Best,
Humberto
On Thu, Jul 31, 2014 at 2:53 PM, Tomasz Figa tomasz.f...@gmail.com wrote:
Hi Humberto,
You can find my comments inline.
On 31.07.2014 13:22, Humberto Silva Naves wrote:
This implements the fixed rate clocks generated either inside or
outside the SoC. It also adds a dt-binding constant for the
sclk_hdmiphy clock, which shall be later used by other drivers,
such as the DRM.
Since the external fixed rate clock fin_pll is now registered by
the clk-exynos5410 file, the bindings with the device tree file have
changed. It is no longer needed to define fin_pll as a fixed clock,
such as in:
fin_pll: xxti {
compatible = fixed-clock;
clock-frequency = 2400;
clock-output-names = fin_pll;
#clock-cells = 0;
};
The above lines should be replaced by the following lines:
fixed-rate-clocks {
oscclk {
compatible = samsung,exynos5410-oscclk;
clock-frequency = 2400;
};
};
This new form of binding was properly documented in the relevant
documentation file.
In general this is backwards. This Exynos-specific clock binding was
invented before generic fixed rate clock binding showed up and so few
drivers still use it to maintain DT ABI compatibility. However new
drivers are required to use the new generic binding and so does the one
for Exynos5410.
Best regards,
Tomasz
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2 4/5] clk: samsung: exynos5410: Add fixed rate clocks
Hi,
On Thu, Jul 31, 2014 at 1:45 PM, Sylwester Nawrocki
s.nawro...@samsung.com wrote:
Can you explain what is rationale behind this change ? Is it related to
suspend/resume ordering ?
I had forgotten, but now remember the reason why I did this. If you
see the current implementation of clk-exynos5410, you will notice it
heavily depends on the clock fin_pll. On the other hand, this clock
exists because in the current dtb (exynos5410-smdk5410.dts), there is
a node fin_pll such as
fin_pll: xxti {
compatible = fixed-clock;
clock-frequency = 2400;
clock-output-names = fin_pll;
#clock-cells = 0;
};
So far so good. But the real problem comes in when I check the rate of
fin_pll to determine if I should install the rate table or not (and I
really need this for my patch). More specifically
if (_get_rate(fin_pll) == 24 * MHZ) {
exynos5410_plls[apll].rate_table = apll_24mhz_tbl;
exynos5410_plls[cpll].rate_table = cpll_24mhz_tbl;
exynos5410_plls[kpll].rate_table = kpll_24mhz_tbl;
exynos5410_plls[dpll].rate_table = dpll_24mhz_tbl;
exynos5410_plls[epll].rate_table = epll_24mhz_tbl;
exynos5410_plls[ipll].rate_table = ipll_24mhz_tbl;
}
I *have* to determine if the rate of fin_pll is 24MHz, and this is
impossible to do if fin_pll is not available. Moreover, there is no
way I can ensure that the fixed clock provider for fin_pll was
initialized before mine, so there is chance that _get_rate won't work.
The only way I fix that is to set the dependency explicitly in the
dtb, by adding the fin_pll clock as required resource.
clock: clock-controller@1001 {
compatible = samsung,exynos5410-clock;
reg = 0x1001 0x3;
#clock-cells = 1;
/* Add the parent clock */
clocks = fin_pll;
clock-names = fin_pll;
};
But in any case, the bindings with the DTB must be changed one way or
another, because I *really* need to use fin_pll on my driver
registration. If you agree with this alternative solution I previously
described, I can change that in the next version of the patch series.
Best regards,
Humberto
Obviously it breaks the kernel/dtb compatibility. We should be moving
in opposite direction, i.e. completely remove the custom samsung fixed
rate clocks.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2 4/5] clk: samsung: exynos5410: Add fixed rate clocks
Humberto,
On 31.07.2014 23:01, Humberto Naves wrote:
Hi,
On Thu, Jul 31, 2014 at 1:45 PM, Sylwester Nawrocki
s.nawro...@samsung.com wrote:
Can you explain what is rationale behind this change ? Is it related to
suspend/resume ordering ?
I had forgotten, but now remember the reason why I did this. If you
see the current implementation of clk-exynos5410, you will notice it
heavily depends on the clock fin_pll. On the other hand, this clock
exists because in the current dtb (exynos5410-smdk5410.dts), there is
a node fin_pll such as
fin_pll: xxti {
compatible = fixed-clock;
clock-frequency = 2400;
clock-output-names = fin_pll;
#clock-cells = 0;
};
So far so good. But the real problem comes in when I check the rate of
fin_pll to determine if I should install the rate table or not (and I
really need this for my patch). More specifically
if (_get_rate(fin_pll) == 24 * MHZ) {
exynos5410_plls[apll].rate_table = apll_24mhz_tbl;
exynos5410_plls[cpll].rate_table = cpll_24mhz_tbl;
exynos5410_plls[kpll].rate_table = kpll_24mhz_tbl;
exynos5410_plls[dpll].rate_table = dpll_24mhz_tbl;
exynos5410_plls[epll].rate_table = epll_24mhz_tbl;
exynos5410_plls[ipll].rate_table = ipll_24mhz_tbl;
}
I *have* to determine if the rate of fin_pll is 24MHz, and this is
impossible to do if fin_pll is not available. Moreover, there is no
way I can ensure that the fixed clock provider for fin_pll was
initialized before mine, so there is chance that _get_rate won't work.
The only way I fix that is to set the dependency explicitly in the
dtb, by adding the fin_pll clock as required resource.
clock: clock-controller@1001 {
compatible = samsung,exynos5410-clock;
reg = 0x1001 0x3;
#clock-cells = 1;
/* Add the parent clock */
clocks = fin_pll;
clock-names = fin_pll;
};
This is the correct solution to your problem. The clocks and clock-names
properties should have been there from the beginning but apparently this
has been missed in review. Also see below.
But in any case, the bindings with the DTB must be changed one way or
another, because I *really* need to use fin_pll on my driver
registration.
This is a backwards compatible change. On DTBs without clocks and
clock-names properties the PLL tables simply won't be registered which
is exactly the same behavior we have now without any tables in the
driver at all.
If you agree with this alternative solution I previously
described, I can change that in the next version of the patch series.
Please update the dts instead, in the way you pointed above.
Best regards,
Tomasz
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
