| Message ID | 1410367054-30926-1-git-send-email-s.nawrocki@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Sylwester, On Wed, Sep 10, 2014 at 10:37 AM, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote: > The default mux and divider clocks are specified in device tree > so that the FIMC devices in Exynos4210 and Exynos4x12 SoCs are > clocked from recommended clock source and with maximum supported > frequency. If needed these settings could be overrode in board > specific dts files, however they are in practice optimal in most > cases. Just curious about the Exynos4x12 situation here. You set the FIMC clocks as 176MHz, child of MPLL, which works for ODROID with a divider: 880MHz MPLL / 5 = 176MHz However, talking of recommended frequencies... Is 880MHz really the standard there? Isn't 800MHz the more common one? Also, if you happen to know, I would be curious about the equivalent and recommended situation for the sclk_mfc clock. On the vendor kernel it is clocked at 880/4 = 220MHz. When booting mainline on an odroid it is 880/16 = 55MHz :/ Thanks Daniel -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Sep 10, 2014 at 10:37 AM, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote: > The default mux and divider clocks are specified in device tree > so that the FIMC devices in Exynos4210 and Exynos4x12 SoCs are > clocked from recommended clock source and with maximum supported > frequency. If needed these settings could be overrode in board > specific dts files, however they are in practice optimal in most > cases. Another consideration for this patch. fimc-core.c already has probe-time code that tries to set the clock frequency, using a clock-frequency property in the DT, and falling back to a hardcoded value in the driver. Maybe it is best to just set the clock rate from one place. Daniel -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Daniel, On 18/09/14 21:27, Daniel Drake wrote: > On Wed, Sep 10, 2014 at 10:37 AM, Sylwester Nawrocki > <s.nawrocki@samsung.com> wrote: >> > The default mux and divider clocks are specified in device tree >> > so that the FIMC devices in Exynos4210 and Exynos4x12 SoCs are >> > clocked from recommended clock source and with maximum supported >> > frequency. If needed these settings could be overrode in board >> > specific dts files, however they are in practice optimal in most >> > cases. > > Just curious about the Exynos4x12 situation here. > You set the FIMC clocks as 176MHz, child of MPLL, which works for > ODROID with a divider: > > 880MHz MPLL / 5 = 176MHz > > However, talking of recommended frequencies... Is 880MHz really the > standard there? > Isn't 800MHz the more common one? AFAIK 880 MHz is recommended MPLL frequency for Exynos4412 EVT2.0, which is revision of the Exynos4412 SoC the Odroid U3 boards are populated with. You can read the main/sub revision information from the chip ID register (at 0x10000000). The frequencies can always be overwritten in board specific dts or DTB could be amended by bootloader if needed. > Also, if you happen to know, I would be curious about the equivalent > and recommended situation for the sclk_mfc clock. On the vendor kernel > it is clocked at 880/4 = 220MHz. When booting mainline on an odroid it > is 880/16 = 55MHz :/ I think we should add similar entry in device tree for the MFC devices. AFAIR now the frequency has fixed value in the driver. I saw some changes in s5p-mfc driver WRT to clock handling recently though, possibly Jacek or Kamil could explain what current situation is. -- Regards, Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 19/09/14 01:53, Daniel Drake wrote: > On Wed, Sep 10, 2014 at 10:37 AM, Sylwester Nawrocki > <s.nawrocki@samsung.com> wrote: >> The default mux and divider clocks are specified in device tree >> so that the FIMC devices in Exynos4210 and Exynos4x12 SoCs are >> clocked from recommended clock source and with maximum supported >> frequency. If needed these settings could be overrode in board >> specific dts files, however they are in practice optimal in most >> cases. > > Another consideration for this patch. > > fimc-core.c already has probe-time code that tries to set the clock > frequency, using a clock-frequency property in the DT, and falling > back to a hardcoded value in the driver. Maybe it is best to just set > the clock rate from one place. Indeed, thanks for pointing this out. I will submit a patch to remove the fallback in fimc-core.c. So the driver doesn't touch both: clock rate and parent setting if 'clock-rates' property is specified in DT. I think clock-frequency property in the FIMC DT binding should be deprecated. So the initial parent clock and frequency settings are handled similarly for all relevant IP blocks, and there is no need to code such things in each individual driver. -- Thanks, Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Sep 25, 2014 at 12:05 PM, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote: >> Just curious about the Exynos4x12 situation here. >> You set the FIMC clocks as 176MHz, child of MPLL, which works for >> ODROID with a divider: >> >> 880MHz MPLL / 5 = 176MHz >> >> However, talking of recommended frequencies... Is 880MHz really the >> standard there? >> Isn't 800MHz the more common one? > > AFAIK 880 MHz is recommended MPLL frequency for Exynos4412 EVT2.0, which > is revision of the Exynos4412 SoC the Odroid U3 boards are populated with. > You can read the main/sub revision information from the chip ID register > (at 0x10000000). Thanks for the information. Might be worth a quick comment in exynos4x12.dtsi then, saying that the defaults here are for Exynos4412 EVT2.0 and might want to be overridden for other versions of the SoC. Thanks Daniel -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Sep 25, 2014 at 1:44 PM, Daniel Drake <drake@endlessm.com> wrote: >> AFAIK 880 MHz is recommended MPLL frequency for Exynos4412 EVT2.0, which >> is revision of the Exynos4412 SoC the Odroid U3 boards are populated with. >> You can read the main/sub revision information from the chip ID register >> (at 0x10000000). > > Thanks for the information. Might be worth a quick comment in > exynos4x12.dtsi then, saying that the defaults here are for Exynos4412 > EVT2.0 and might want to be overridden for other versions of the SoC. Also can you double check this? The old ODROID-X is Exynos4412 EVT1.0 and runs 880MHz MPLL. Daniel -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Sylwester, On 10.09.2014 18:37, Sylwester Nawrocki wrote: > The default mux and divider clocks are specified in device tree > so that the FIMC devices in Exynos4210 and Exynos4x12 SoCs are > clocked from recommended clock source and with maximum supported > frequency. If needed these settings could be overrode in board > specific dts files, however they are in practice optimal in most > cases. > > Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > --- > arch/arm/boot/dts/exynos4210.dtsi | 16 ++++++++++++++++ > arch/arm/boot/dts/exynos4x12.dtsi | 16 ++++++++++++++++ > 2 files changed, 32 insertions(+) > > diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi > index 807bb5b..0969d2e 100644 > --- a/arch/arm/boot/dts/exynos4210.dtsi > +++ b/arch/arm/boot/dts/exynos4210.dtsi > @@ -154,18 +154,30 @@ > samsung,pix-limits = <4224 8192 1920 4224>; > samsung,mainscaler-ext; > samsung,cam-if; > + assigned-clocks = <&clock CLK_MOUT_FIMC0>, > + <&clock CLK_SCLK_FIMC0>; > + assigned-clock-parents = <&clock CLK_SCLK_MPLL>; > + assigned-clock-rates = <0>, <160000000>; I wonder whether such settings shouldn't really be set up on a per-board basis. As Daniel already pointed, we have cases when MPLL frequency differs across boards, but we might also have boards that differ in power budget and so having different desired operating frequencies for various IP blocks. What do you think? Best regards, Tomasz -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Tomasz, On 25/09/14 23:58, Tomasz Figa wrote: > On 10.09.2014 18:37, Sylwester Nawrocki wrote: >> > The default mux and divider clocks are specified in device tree >> > so that the FIMC devices in Exynos4210 and Exynos4x12 SoCs are >> > clocked from recommended clock source and with maximum supported >> > frequency. If needed these settings could be overrode in board >> > specific dts files, however they are in practice optimal in most >> > cases. >> > >> > Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> >> > --- >> > arch/arm/boot/dts/exynos4210.dtsi | 16 ++++++++++++++++ >> > arch/arm/boot/dts/exynos4x12.dtsi | 16 ++++++++++++++++ >> > 2 files changed, 32 insertions(+) >> > >> > diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi >> > index 807bb5b..0969d2e 100644 >> > --- a/arch/arm/boot/dts/exynos4210.dtsi >> > +++ b/arch/arm/boot/dts/exynos4210.dtsi >> > @@ -154,18 +154,30 @@ >> > samsung,pix-limits = <4224 8192 1920 4224>; >> > samsung,mainscaler-ext; >> > samsung,cam-if; >> > + assigned-clocks = <&clock CLK_MOUT_FIMC0>, >> > + <&clock CLK_SCLK_FIMC0>; >> > + assigned-clock-parents = <&clock CLK_SCLK_MPLL>; >> > + assigned-clock-rates = <0>, <160000000>; > > I wonder whether such settings shouldn't really be set up on a per-board > basis. > > As Daniel already pointed, we have cases when MPLL frequency differs > across boards, but we might also have boards that differ in power budget > and so having different desired operating frequencies for various IP blocks. > > What do you think? This patch provides sane default values for Exynos4210, MPLL is recommended clock source for FIMC devices. If any other clock frequency is needed for selected boards the clocks setup could be simply overwritten in board dts file. Otherwise similar changes would have to be done in each board dts. Alternatively I could split it and leave only parent clock assignment in SoC dts, moving assigned-clock-rates properties to board dts. I'm going to leave the functional clock frequency setting in the driver as it is done now, and to just modify the fallback to driver data, to have also 'assigned-clock-rates' considered in the driver. So parent clock assignment independently of the IP block driver in dts, and the functional clock frequency set in the driver from driver data.
On 26.09.2014 13:01, Sylwester Nawrocki wrote: > Hi Tomasz, > > On 25/09/14 23:58, Tomasz Figa wrote: >> On 10.09.2014 18:37, Sylwester Nawrocki wrote: >>>> The default mux and divider clocks are specified in device tree >>>> so that the FIMC devices in Exynos4210 and Exynos4x12 SoCs are >>>> clocked from recommended clock source and with maximum supported >>>> frequency. If needed these settings could be overrode in board >>>> specific dts files, however they are in practice optimal in most >>>> cases. >>>> >>>> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> >>>> --- >>>> arch/arm/boot/dts/exynos4210.dtsi | 16 ++++++++++++++++ >>>> arch/arm/boot/dts/exynos4x12.dtsi | 16 ++++++++++++++++ >>>> 2 files changed, 32 insertions(+) >>>> >>>> diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi >>>> index 807bb5b..0969d2e 100644 >>>> --- a/arch/arm/boot/dts/exynos4210.dtsi >>>> +++ b/arch/arm/boot/dts/exynos4210.dtsi >>>> @@ -154,18 +154,30 @@ >>>> samsung,pix-limits = <4224 8192 1920 4224>; >>>> samsung,mainscaler-ext; >>>> samsung,cam-if; >>>> + assigned-clocks = <&clock CLK_MOUT_FIMC0>, >>>> + <&clock CLK_SCLK_FIMC0>; >>>> + assigned-clock-parents = <&clock CLK_SCLK_MPLL>; >>>> + assigned-clock-rates = <0>, <160000000>; >> >> I wonder whether such settings shouldn't really be set up on a per-board >> basis. >> >> As Daniel already pointed, we have cases when MPLL frequency differs >> across boards, but we might also have boards that differ in power budget >> and so having different desired operating frequencies for various IP blocks. >> >> What do you think? > > This patch provides sane default values for Exynos4210, MPLL is recommended > clock source for FIMC devices. If any other clock frequency is needed for > selected boards the clocks setup could be simply overwritten in board dts > file. Otherwise similar changes would have to be done in each board dts. I'm not concerned specifically with Exynos4210, but with placing such kind of data in common dtsi files. Notice that even on boards which have correct initialization done by firmware, this will cause the settings to be overwritten, even if the firmware sets correct, but different values, regardless of them being clock parents or rates. To me, even if this would mean duplicating some data, making this per board and present only in dts files of boards that actually need this (i.e. are known to have broken firmware) sounds more reasonable. Best regards, Tomasz -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 26/09/14 15:24, Tomasz Figa wrote: > I'm not concerned specifically with Exynos4210, but with placing such > kind of data in common dtsi files. > > Notice that even on boards which have correct initialization done by > firmware, this will cause the settings to be overwritten, even if the > firmware sets correct, but different values, regardless of them being > clock parents or rates. > > To me, even if this would mean duplicating some data, making this per > board and present only in dts files of boards that actually need this > (i.e. are known to have broken firmware) sounds more reasonable. OTOH by having those settings in device tree would ensure the clock tree is set correctly, regardless of the firmware. There is only one correct clock parent for these devices, so the kernel couldn't do any harm. I'd say it never worked in practice to rely on the bootloader to configure these things, and one could say the firmware has been broken in general with regards to this issue. -- Regards, Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi index 807bb5b..0969d2e 100644 --- a/arch/arm/boot/dts/exynos4210.dtsi +++ b/arch/arm/boot/dts/exynos4210.dtsi @@ -154,18 +154,30 @@ samsung,pix-limits = <4224 8192 1920 4224>; samsung,mainscaler-ext; samsung,cam-if; + assigned-clocks = <&clock CLK_MOUT_FIMC0>, + <&clock CLK_SCLK_FIMC0>; + assigned-clock-parents = <&clock CLK_SCLK_MPLL>; + assigned-clock-rates = <0>, <160000000>; }; fimc_1: fimc@11810000 { samsung,pix-limits = <4224 8192 1920 4224>; samsung,mainscaler-ext; samsung,cam-if; + assigned-clocks = <&clock CLK_MOUT_FIMC1>, + <&clock CLK_SCLK_FIMC1>; + assigned-clock-parents = <&clock CLK_SCLK_MPLL>; + assigned-clock-rates = <0>, <160000000>; }; fimc_2: fimc@11820000 { samsung,pix-limits = <4224 8192 1920 4224>; samsung,mainscaler-ext; samsung,lcd-wb; + assigned-clocks = <&clock CLK_MOUT_FIMC2>, + <&clock CLK_SCLK_FIMC2>; + assigned-clock-parents = <&clock CLK_SCLK_MPLL>; + assigned-clock-rates = <0>, <160000000>; }; fimc_3: fimc@11830000 { @@ -173,6 +185,10 @@ samsung,rotators = <0>; samsung,mainscaler-ext; samsung,lcd-wb; + assigned-clocks = <&clock CLK_MOUT_FIMC3>, + <&clock CLK_SCLK_FIMC3>; + assigned-clock-parents = <&clock CLK_SCLK_MPLL>; + assigned-clock-rates = <0>, <160000000>; }; }; }; diff --git a/arch/arm/boot/dts/exynos4x12.dtsi b/arch/arm/boot/dts/exynos4x12.dtsi index 861bb91..38ba14f 100644 --- a/arch/arm/boot/dts/exynos4x12.dtsi +++ b/arch/arm/boot/dts/exynos4x12.dtsi @@ -162,6 +162,10 @@ samsung,mainscaler-ext; samsung,isp-wb; samsung,cam-if; + assigned-clocks = <&clock CLK_MOUT_FIMC0>, + <&clock CLK_SCLK_FIMC0>; + assigned-clock-parents = <&clock CLK_MOUT_MPLL_USER_T>; + assigned-clock-rates = <0>, <176000000>; }; fimc_1: fimc@11810000 { @@ -170,6 +174,10 @@ samsung,mainscaler-ext; samsung,isp-wb; samsung,cam-if; + assigned-clocks = <&clock CLK_MOUT_FIMC1>, + <&clock CLK_SCLK_FIMC1>; + assigned-clock-parents = <&clock CLK_MOUT_MPLL_USER_T>; + assigned-clock-rates = <0>, <176000000>; }; fimc_2: fimc@11820000 { @@ -179,6 +187,10 @@ samsung,isp-wb; samsung,lcd-wb; samsung,cam-if; + assigned-clocks = <&clock CLK_MOUT_FIMC2>, + <&clock CLK_SCLK_FIMC2>; + assigned-clock-parents = <&clock CLK_MOUT_MPLL_USER_T>; + assigned-clock-rates = <0>, <176000000>; }; fimc_3: fimc@11830000 { @@ -188,6 +200,10 @@ samsung,mainscaler-ext; samsung,isp-wb; samsung,lcd-wb; + assigned-clocks = <&clock CLK_MOUT_FIMC3>, + <&clock CLK_SCLK_FIMC3>; + assigned-clock-parents = <&clock CLK_MOUT_MPLL_USER_T>; + assigned-clock-rates = <0>, <176000000>; }; fimc_lite_0: fimc-lite@12390000 {
The default mux and divider clocks are specified in device tree so that the FIMC devices in Exynos4210 and Exynos4x12 SoCs are clocked from recommended clock source and with maximum supported frequency. If needed these settings could be overrode in board specific dts files, however they are in practice optimal in most cases. Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> --- arch/arm/boot/dts/exynos4210.dtsi | 16 ++++++++++++++++ arch/arm/boot/dts/exynos4x12.dtsi | 16 ++++++++++++++++ 2 files changed, 32 insertions(+)