Discussion:
[PATCH 3.4] sunxi: Calculate PLL5P clock divisors for G2D, ACE and DEBE
Siarhei Siamashka
2014-10-04 05:53:57 UTC
Permalink
When PLL5P is used as a parent clock for some of the peripherals,
the current code just selects some hardcoded divisors. This happens
to work, but only under assumption that the PLL5P clock speed is
somewhere between 360MHz and 480MHz (the typical DRAM clock speeds).

However with some tweaks for the DRAM parameters, it is possible to
clock DRAM up to 600MHz and more on some devices:

http://lists.denx.de/pipermail/u-boot/2014-July/183981.html

And this introduces concerns about the hardcoded divisors in the
kernel, which may cause some peripherals to operate at abnormally
high clock speeds if the PLL5 clock speed is too fast (PLL5 is used
for clocking DRAM).

Moreover, it makes sense to avoid pre-dividing PLL5P and make it run
even faster than DRAM. This provides better granularity of the clock
speed selection for MBUS, G2D and everything else that is using PLL5P
as the parent clock. but running PLL5P faster means that the hardcoded
divisors become even more inappropriate.

This patch improves the clock divisors selection for G2D, ACE and
DEBE to insure that they can work correctly with any PLL5P clock
speed.

Signed-off-by: Siarhei Siamashka <siarhei.siamashka-***@public.gmane.org>
---
drivers/char/sunxi_g2d/g2d.c | 8 ++++++--
drivers/media/audio/sun4i_dev_ace.c | 7 ++++++-
drivers/video/sunxi/disp/disp_clk.c | 17 ++++++++---------
3 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/drivers/char/sunxi_g2d/g2d.c b/drivers/char/sunxi_g2d/g2d.c
index 288685a..539c726 100644
--- a/drivers/char/sunxi_g2d/g2d.c
+++ b/drivers/char/sunxi_g2d/g2d.c
@@ -29,9 +29,12 @@
struct clk *g2d_ahbclk,*g2d_dramclk,*g2d_mclk,*g2d_src;
extern __g2d_drv_t g2d_ext_hd;

+/* Arbitrarily pick 240MHz (TODO: confirm what is the real limit) */
+#define G2D_CLOCK_SPEED_LIMIT 240000000
+
int g2d_openclk(void)
{
- __u32 ret;
+ __u32 ret, g2d_div;

/* ahb g2d gating */
g2d_ahbclk = clk_get(NULL,"ahb_de_mix");
@@ -51,7 +54,8 @@ int g2d_openclk(void)
clk_put(g2d_src);

ret = clk_get_rate(g2d_src);
- clk_set_rate(g2d_mclk,ret/2);
+ g2d_div = DIV_ROUND_UP(ret, G2D_CLOCK_SPEED_LIMIT);
+ clk_set_rate(g2d_mclk, ret / g2d_div);

return 0;
}
diff --git a/drivers/media/audio/sun4i_dev_ace.c b/drivers/media/audio/sun4i_dev_ace.c
index 59fdfeb..e3599f6 100644
--- a/drivers/media/audio/sun4i_dev_ace.c
+++ b/drivers/media/audio/sun4i_dev_ace.c
@@ -91,10 +91,14 @@ static irqreturn_t ace_interrupt(int irq, void *dev)
return IRQ_HANDLED;
}

+/* 200MHz is the limit for ACE_CLK_REG (see the A10 User Manual) */
+#define ACE_CLOCK_SPEED_LIMIT 200000000
+
static long ace_dev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
{
int ret_val = 0;
unsigned long test_arg;
+ int pll5_div;
__ace_req_e mpara;
unsigned long rate;
switch (cmd){
@@ -137,7 +141,8 @@ static long ace_dev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg
printk("try to set parent of ace_moduleclk to ace_pll5clk failed!\n");
}
rate = clk_get_rate(ace_pll5_pclk);
- if(clk_set_rate(ace_moduleclk, rate/2)) {
+ pll5_div = DIV_ROUND_UP(rate, ACE_CLOCK_SPEED_LIMIT);
+ if(clk_set_rate(ace_moduleclk, rate / pll5_div)) {
printk("try to set ace_moduleclk rate failed!!!\n");
goto out;
}
diff --git a/drivers/video/sunxi/disp/disp_clk.c b/drivers/video/sunxi/disp/disp_clk.c
index abd1877..ff46bcc 100644
--- a/drivers/video/sunxi/disp/disp_clk.c
+++ b/drivers/video/sunxi/disp/disp_clk.c
@@ -120,9 +120,12 @@ __disp_clk_tab clk_tab = {
}
};

+/* 300MHz */
+#define DEBE_CLOCK_SPEED_LIMIT 300000000
+
__s32 image_clk_init(__u32 sel)
{
- __u32 dram_pll;
+ __u32 dram_pll, pll5_div;

if (sel == 0) {
h_debe0ahbclk = OSAL_CCMU_OpenMclk(AW_MOD_CLK_AHB_DEBE0);
@@ -137,10 +140,8 @@ __s32 image_clk_init(__u32 sel)
OSAL_CCMU_SetMclkSrc(h_debe0mclk, AW_SYS_CLK_PLL5P);

dram_pll = OSAL_CCMU_GetSrcFreq(AW_SYS_CLK_PLL5P);
- if (dram_pll < 300000000)
- OSAL_CCMU_SetMclkDiv(h_debe0mclk, 1);
- else
- OSAL_CCMU_SetMclkDiv(h_debe0mclk, 2);
+ pll5_div = DIV_ROUND_UP(dram_pll, DEBE_CLOCK_SPEED_LIMIT);
+ OSAL_CCMU_SetMclkDiv(h_debe0mclk, pll5_div);

OSAL_CCMU_MclkOnOff(h_debe0ahbclk, CLK_ON);
if (sunxi_is_sun4i()) {
@@ -162,10 +163,8 @@ __s32 image_clk_init(__u32 sel)
OSAL_CCMU_SetMclkSrc(h_debe1mclk, AW_SYS_CLK_PLL5P);

dram_pll = OSAL_CCMU_GetSrcFreq(AW_SYS_CLK_PLL5P);
- if (dram_pll < 300000000)
- OSAL_CCMU_SetMclkDiv(h_debe1mclk, 1);
- else
- OSAL_CCMU_SetMclkDiv(h_debe1mclk, 2);
+ pll5_div = DIV_ROUND_UP(dram_pll, DEBE_CLOCK_SPEED_LIMIT);
+ OSAL_CCMU_SetMclkDiv(h_debe1mclk, pll5_div);

OSAL_CCMU_MclkOnOff(h_debe1ahbclk, CLK_ON);
if (sunxi_is_sun4i()) {
--
1.8.3.2
Hans de Goede
2014-10-13 11:09:59 UTC
Permalink
Hi,
The dcdc3 voltage is expected to be set by the bootloader and the right
voltage depends on the DRAM settings (higher clock speed needs more
voltage). Allowing to use the 'dcdc3_vol' parameter from the FEX file
only introduces an unnecessary fragile dependency between the settings
in u-boot and the settings in FEX. So now we ignore this parameter in
FEX and keep the original dcdc3 voltage set by the bootloader.
[ 2.212941] axp20_ldo1: 1300 mV
[ 2.221370] axp20_ldo2: 1800 <--> 3300 mV at 3000 mV
[ 2.231662] axp20_ldo3: 700 <--> 3500 mV at 2800 mV
[ 2.241747] axp20_ldo4: 1250 <--> 3300 mV at 2800 mV
[ 2.251906] axp20_buck2: 700 <--> 2275 mV at 1400 mV
[ 2.263430] somebody is trying to set dcdc3 range to (1300000, 1300000) uV
[ 2.275327] but we keep dcdc3 = 1250000 uV from the bootloader
[ 2.285406] axp20_buck3: 700 <--> 3500 mV at 1250 mV
[ 2.295299] axp20_ldoio0: 1800 <--> 3300 mV at 2800 mV
Looks good:

Acked-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+***@public.gmane.org>

Can we please get this merged, I'm working on getting the sunxi-3.4 kernels
to work with upstream u-boot, so that we can stop maintaining our own fork,
and this is necessary for this.

Regards,

Hans
---
drivers/power/axp_power/axp20-regu.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/power/axp_power/axp20-regu.c b/drivers/power/axp_power/axp20-regu.c
index e56c5f5..30ceb5c 100644
--- a/drivers/power/axp_power/axp20-regu.c
+++ b/drivers/power/axp_power/axp20-regu.c
@@ -37,6 +37,7 @@ static inline int check_range(struct axp_regulator_info *info,
return 0;
}
+static int axp_get_voltage(struct regulator_dev *rdev);
/* AXP common operations */
static int axp_set_voltage(struct regulator_dev *rdev, int min_uV, int max_uV,
@@ -45,7 +46,14 @@ static int axp_set_voltage(struct regulator_dev *rdev, int min_uV, int max_uV,
struct axp_regulator_info *info = rdev_get_drvdata(rdev);
struct device *axp_dev = to_axp_dev(rdev);
uint8_t val, mask;
-
+
+ if (rdev_get_id(rdev) == AXP20_ID_BUCK3) {
+ pr_err("somebody is trying to set dcdc3 range to (%d, %d) uV\n",
+ min_uV, max_uV);
+ pr_err("but we keep dcdc3 = %d uV from the bootloader\n",
+ axp_get_voltage(rdev));
+ return 0;
+ }
if (check_range(info, min_uV, max_uV)) {
pr_err("invalid voltage range (%d, %d) uV\n", min_uV, max_uV);
Siarhei Siamashka
2014-10-14 21:27:23 UTC
Permalink
On Mon, 13 Oct 2014 13:09:59 +0200
Hi,
The dcdc3 voltage is expected to be set by the bootloader and the right
voltage depends on the DRAM settings (higher clock speed needs more
voltage). Allowing to use the 'dcdc3_vol' parameter from the FEX file
only introduces an unnecessary fragile dependency between the settings
in u-boot and the settings in FEX. So now we ignore this parameter in
FEX and keep the original dcdc3 voltage set by the bootloader.
[ 2.212941] axp20_ldo1: 1300 mV
[ 2.221370] axp20_ldo2: 1800 <--> 3300 mV at 3000 mV
[ 2.231662] axp20_ldo3: 700 <--> 3500 mV at 2800 mV
[ 2.241747] axp20_ldo4: 1250 <--> 3300 mV at 2800 mV
[ 2.251906] axp20_buck2: 700 <--> 2275 mV at 1400 mV
[ 2.263430] somebody is trying to set dcdc3 range to (1300000, 1300000) uV
[ 2.275327] but we keep dcdc3 = 1250000 uV from the bootloader
[ 2.285406] axp20_buck3: 700 <--> 3500 mV at 1250 mV
[ 2.295299] axp20_ldoio0: 1800 <--> 3300 mV at 2800 mV
Thanks for the review. Pushed to stage/sunxi-3.4
--
Best regards,
Siarhei Siamashka
Hans de Goede
2014-10-13 11:10:28 UTC
Permalink
Hi,
Post by Siarhei Siamashka
When PLL5P is used as a parent clock for some of the peripherals,
the current code just selects some hardcoded divisors. This happens
to work, but only under assumption that the PLL5P clock speed is
somewhere between 360MHz and 480MHz (the typical DRAM clock speeds).
However with some tweaks for the DRAM parameters, it is possible to
http://lists.denx.de/pipermail/u-boot/2014-July/183981.html
And this introduces concerns about the hardcoded divisors in the
kernel, which may cause some peripherals to operate at abnormally
high clock speeds if the PLL5 clock speed is too fast (PLL5 is used
for clocking DRAM).
Moreover, it makes sense to avoid pre-dividing PLL5P and make it run
even faster than DRAM. This provides better granularity of the clock
speed selection for MBUS, G2D and everything else that is using PLL5P
as the parent clock. but running PLL5P faster means that the hardcoded
divisors become even more inappropriate.
This patch improves the clock divisors selection for G2D, ACE and
DEBE to insure that they can work correctly with any PLL5P clock
speed.
Looks good:

Acked-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+***@public.gmane.org>

Can we please get this merged, I'm working on getting the sunxi-3.4 kernels
to work with upstream u-boot, so that we can stop maintaining our own fork,
and this is necessary for this.

Regards,

Hans
Post by Siarhei Siamashka
---
drivers/char/sunxi_g2d/g2d.c | 8 ++++++--
drivers/media/audio/sun4i_dev_ace.c | 7 ++++++-
drivers/video/sunxi/disp/disp_clk.c | 17 ++++++++---------
3 files changed, 20 insertions(+), 12 deletions(-)
diff --git a/drivers/char/sunxi_g2d/g2d.c b/drivers/char/sunxi_g2d/g2d.c
index 288685a..539c726 100644
--- a/drivers/char/sunxi_g2d/g2d.c
+++ b/drivers/char/sunxi_g2d/g2d.c
@@ -29,9 +29,12 @@
struct clk *g2d_ahbclk,*g2d_dramclk,*g2d_mclk,*g2d_src;
extern __g2d_drv_t g2d_ext_hd;
+/* Arbitrarily pick 240MHz (TODO: confirm what is the real limit) */
+#define G2D_CLOCK_SPEED_LIMIT 240000000
+
int g2d_openclk(void)
{
- __u32 ret;
+ __u32 ret, g2d_div;
/* ahb g2d gating */
g2d_ahbclk = clk_get(NULL,"ahb_de_mix");
@@ -51,7 +54,8 @@ int g2d_openclk(void)
clk_put(g2d_src);
ret = clk_get_rate(g2d_src);
- clk_set_rate(g2d_mclk,ret/2);
+ g2d_div = DIV_ROUND_UP(ret, G2D_CLOCK_SPEED_LIMIT);
+ clk_set_rate(g2d_mclk, ret / g2d_div);
return 0;
}
diff --git a/drivers/media/audio/sun4i_dev_ace.c b/drivers/media/audio/sun4i_dev_ace.c
index 59fdfeb..e3599f6 100644
--- a/drivers/media/audio/sun4i_dev_ace.c
+++ b/drivers/media/audio/sun4i_dev_ace.c
@@ -91,10 +91,14 @@ static irqreturn_t ace_interrupt(int irq, void *dev)
return IRQ_HANDLED;
}
+/* 200MHz is the limit for ACE_CLK_REG (see the A10 User Manual) */
+#define ACE_CLOCK_SPEED_LIMIT 200000000
+
static long ace_dev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
{
int ret_val = 0;
unsigned long test_arg;
+ int pll5_div;
__ace_req_e mpara;
unsigned long rate;
switch (cmd){
@@ -137,7 +141,8 @@ static long ace_dev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg
printk("try to set parent of ace_moduleclk to ace_pll5clk failed!\n");
}
rate = clk_get_rate(ace_pll5_pclk);
- if(clk_set_rate(ace_moduleclk, rate/2)) {
+ pll5_div = DIV_ROUND_UP(rate, ACE_CLOCK_SPEED_LIMIT);
+ if(clk_set_rate(ace_moduleclk, rate / pll5_div)) {
printk("try to set ace_moduleclk rate failed!!!\n");
goto out;
}
diff --git a/drivers/video/sunxi/disp/disp_clk.c b/drivers/video/sunxi/disp/disp_clk.c
index abd1877..ff46bcc 100644
--- a/drivers/video/sunxi/disp/disp_clk.c
+++ b/drivers/video/sunxi/disp/disp_clk.c
@@ -120,9 +120,12 @@ __disp_clk_tab clk_tab = {
}
};
+/* 300MHz */
+#define DEBE_CLOCK_SPEED_LIMIT 300000000
+
__s32 image_clk_init(__u32 sel)
{
- __u32 dram_pll;
+ __u32 dram_pll, pll5_div;
if (sel == 0) {
h_debe0ahbclk = OSAL_CCMU_OpenMclk(AW_MOD_CLK_AHB_DEBE0);
@@ -137,10 +140,8 @@ __s32 image_clk_init(__u32 sel)
OSAL_CCMU_SetMclkSrc(h_debe0mclk, AW_SYS_CLK_PLL5P);
dram_pll = OSAL_CCMU_GetSrcFreq(AW_SYS_CLK_PLL5P);
- if (dram_pll < 300000000)
- OSAL_CCMU_SetMclkDiv(h_debe0mclk, 1);
- else
- OSAL_CCMU_SetMclkDiv(h_debe0mclk, 2);
+ pll5_div = DIV_ROUND_UP(dram_pll, DEBE_CLOCK_SPEED_LIMIT);
+ OSAL_CCMU_SetMclkDiv(h_debe0mclk, pll5_div);
OSAL_CCMU_MclkOnOff(h_debe0ahbclk, CLK_ON);
if (sunxi_is_sun4i()) {
@@ -162,10 +163,8 @@ __s32 image_clk_init(__u32 sel)
OSAL_CCMU_SetMclkSrc(h_debe1mclk, AW_SYS_CLK_PLL5P);
dram_pll = OSAL_CCMU_GetSrcFreq(AW_SYS_CLK_PLL5P);
- if (dram_pll < 300000000)
- OSAL_CCMU_SetMclkDiv(h_debe1mclk, 1);
- else
- OSAL_CCMU_SetMclkDiv(h_debe1mclk, 2);
+ pll5_div = DIV_ROUND_UP(dram_pll, DEBE_CLOCK_SPEED_LIMIT);
+ OSAL_CCMU_SetMclkDiv(h_debe1mclk, pll5_div);
OSAL_CCMU_MclkOnOff(h_debe1ahbclk, CLK_ON);
if (sunxi_is_sun4i()) {
Siarhei Siamashka
2014-10-14 21:25:39 UTC
Permalink
On Mon, 13 Oct 2014 13:10:28 +0200
Post by Hans de Goede
Hi,
Post by Siarhei Siamashka
When PLL5P is used as a parent clock for some of the peripherals,
the current code just selects some hardcoded divisors. This happens
to work, but only under assumption that the PLL5P clock speed is
somewhere between 360MHz and 480MHz (the typical DRAM clock speeds).
However with some tweaks for the DRAM parameters, it is possible to
http://lists.denx.de/pipermail/u-boot/2014-July/183981.html
And this introduces concerns about the hardcoded divisors in the
kernel, which may cause some peripherals to operate at abnormally
high clock speeds if the PLL5 clock speed is too fast (PLL5 is used
for clocking DRAM).
Moreover, it makes sense to avoid pre-dividing PLL5P and make it run
even faster than DRAM. This provides better granularity of the clock
speed selection for MBUS, G2D and everything else that is using PLL5P
as the parent clock. but running PLL5P faster means that the hardcoded
divisors become even more inappropriate.
This patch improves the clock divisors selection for G2D, ACE and
DEBE to insure that they can work correctly with any PLL5P clock
speed.
Can we please get this merged,
You know the rules. Every patch needs to be approved by at least one
person other than the submitter.

Thanks for the review. Pushed to stage/sunxi-3.4
Post by Hans de Goede
I'm working on getting the sunxi-3.4 kernels to work with upstream
u-boot, so that we can stop maintaining our own fork,
and this is necessary for this.
I hope we don't get in each other's way with this stuff.
--
Best regards,
Siarhei Siamashka
Hans de Goede
2014-10-15 10:04:13 UTC
Permalink
Hi,
Post by Siarhei Siamashka
On Mon, 13 Oct 2014 13:10:28 +0200
Post by Hans de Goede
Hi,
Post by Siarhei Siamashka
When PLL5P is used as a parent clock for some of the peripherals,
the current code just selects some hardcoded divisors. This happens
to work, but only under assumption that the PLL5P clock speed is
somewhere between 360MHz and 480MHz (the typical DRAM clock speeds).
However with some tweaks for the DRAM parameters, it is possible to
http://lists.denx.de/pipermail/u-boot/2014-July/183981.html
And this introduces concerns about the hardcoded divisors in the
kernel, which may cause some peripherals to operate at abnormally
high clock speeds if the PLL5 clock speed is too fast (PLL5 is used
for clocking DRAM).
Moreover, it makes sense to avoid pre-dividing PLL5P and make it run
even faster than DRAM. This provides better granularity of the clock
speed selection for MBUS, G2D and everything else that is using PLL5P
as the parent clock. but running PLL5P faster means that the hardcoded
divisors become even more inappropriate.
This patch improves the clock divisors selection for G2D, ACE and
DEBE to insure that they can work correctly with any PLL5P clock
speed.
Can we please get this merged,
You know the rules. Every patch needs to be approved by at least one
person other than the submitter.
Thanks for the review. Pushed to stage/sunxi-3.4
Post by Hans de Goede
I'm working on getting the sunxi-3.4 kernels to work with upstream
u-boot, so that we can stop maintaining our own fork,
and this is necessary for this.
I hope we don't get in each other's way with this stuff.
I hope so too :)

I'm actually more or less done now, I've just send out 2 sunxi-3.4 patches
(which are quite similar to your 2, but for slightly different areas),
which completes my sunxi-3.4 work.

With these patches the 3.4 kernels will work on sun4i / sun5i with
an unmodified upstream u-boot. Still I've decided to revert your
PLL5 changes in upstream u-boot (for now) as I really want people to
be able to run unmodified android kernels on the upstream u-boot.

My main reason for this is that I want to take away any reasons
people may have to keep using the linux-sunxi/u-boot-sunxi u-boot "fork",
so that we can focus all u-boot work upstream.

Besides the revert I've also been working on solving the A20 problem,
I'm currently at ELCE, and I've been talking to various u-boot
people about this, including Marc Zyngier, and we've come up with a
solution which should be acceptable for upstream inclusion.

See the 2 u-boot patches I'm about to send for details. I'll put you in
the Cc of them.

About the pll5 settings revert, if you disagree, please reply to the
patch I'm about to send for this, as the proper venue to discuss that
is the upstream u-boot list.

Regards,

Hans
Julian Calaby
2014-10-15 22:44:31 UTC
Permalink
Hi Hans,
Post by Hans de Goede
Hi,
Post by Siarhei Siamashka
On Mon, 13 Oct 2014 13:10:28 +0200
Post by Hans de Goede
Hi,
Post by Siarhei Siamashka
When PLL5P is used as a parent clock for some of the peripherals,
the current code just selects some hardcoded divisors. This happens
to work, but only under assumption that the PLL5P clock speed is
somewhere between 360MHz and 480MHz (the typical DRAM clock speeds).
However with some tweaks for the DRAM parameters, it is possible to
http://lists.denx.de/pipermail/u-boot/2014-July/183981.html
And this introduces concerns about the hardcoded divisors in the
kernel, which may cause some peripherals to operate at abnormally
high clock speeds if the PLL5 clock speed is too fast (PLL5 is used
for clocking DRAM).
Moreover, it makes sense to avoid pre-dividing PLL5P and make it run
even faster than DRAM. This provides better granularity of the clock
speed selection for MBUS, G2D and everything else that is using PLL5P
as the parent clock. but running PLL5P faster means that the hardcoded
divisors become even more inappropriate.
This patch improves the clock divisors selection for G2D, ACE and
DEBE to insure that they can work correctly with any PLL5P clock
speed.
Can we please get this merged,
You know the rules. Every patch needs to be approved by at least one
person other than the submitter.
Thanks for the review. Pushed to stage/sunxi-3.4
Post by Hans de Goede
I'm working on getting the sunxi-3.4 kernels to work with upstream
u-boot, so that we can stop maintaining our own fork,
and this is necessary for this.
I hope we don't get in each other's way with this stuff.
I hope so too :)
I'm actually more or less done now, I've just send out 2 sunxi-3.4 patches
(which are quite similar to your 2, but for slightly different areas),
which completes my sunxi-3.4 work.
With these patches the 3.4 kernels will work on sun4i / sun5i with
an unmodified upstream u-boot. Still I've decided to revert your
PLL5 changes in upstream u-boot (for now) as I really want people to
be able to run unmodified android kernels on the upstream u-boot.
My main reason for this is that I want to take away any reasons
people may have to keep using the linux-sunxi/u-boot-sunxi u-boot "fork",
so that we can focus all u-boot work upstream.
I assume this means that u-boot-sunxi is going away (or at least only
being kept for historical reasons)

A couple of quick questions:

1. What happens to all the device memory info / configuration we have
in our "fork" of u-boot? I assume that'll all have to get ported
across at some point. (I don't have the code in front of me so if this
has already happened, then please ignore this)

2. I produced a script to detect duplicate dram_*.c files, will this
be useful upstream?

Thanks,
--
Julian Calaby

Email: julian.calaby-***@public.gmane.org
Profile: http://www.google.com/profiles/julian.calaby/
Hans de Goede
2014-10-16 08:23:42 UTC
Permalink
Hi,
Post by Julian Calaby
Hi Hans,
Post by Hans de Goede
Hi,
Post by Siarhei Siamashka
On Mon, 13 Oct 2014 13:10:28 +0200
Post by Hans de Goede
Hi,
Post by Siarhei Siamashka
When PLL5P is used as a parent clock for some of the peripherals,
the current code just selects some hardcoded divisors. This happens
to work, but only under assumption that the PLL5P clock speed is
somewhere between 360MHz and 480MHz (the typical DRAM clock speeds).
However with some tweaks for the DRAM parameters, it is possible to
http://lists.denx.de/pipermail/u-boot/2014-July/183981.html
And this introduces concerns about the hardcoded divisors in the
kernel, which may cause some peripherals to operate at abnormally
high clock speeds if the PLL5 clock speed is too fast (PLL5 is used
for clocking DRAM).
Moreover, it makes sense to avoid pre-dividing PLL5P and make it run
even faster than DRAM. This provides better granularity of the clock
speed selection for MBUS, G2D and everything else that is using PLL5P
as the parent clock. but running PLL5P faster means that the hardcoded
divisors become even more inappropriate.
This patch improves the clock divisors selection for G2D, ACE and
DEBE to insure that they can work correctly with any PLL5P clock
speed.
Can we please get this merged,
You know the rules. Every patch needs to be approved by at least one
person other than the submitter.
Thanks for the review. Pushed to stage/sunxi-3.4
Post by Hans de Goede
I'm working on getting the sunxi-3.4 kernels to work with upstream
u-boot, so that we can stop maintaining our own fork,
and this is necessary for this.
I hope we don't get in each other's way with this stuff.
I hope so too :)
I'm actually more or less done now, I've just send out 2 sunxi-3.4 patches
(which are quite similar to your 2, but for slightly different areas),
which completes my sunxi-3.4 work.
With these patches the 3.4 kernels will work on sun4i / sun5i with
an unmodified upstream u-boot. Still I've decided to revert your
PLL5 changes in upstream u-boot (for now) as I really want people to
be able to run unmodified android kernels on the upstream u-boot.
My main reason for this is that I want to take away any reasons
people may have to keep using the linux-sunxi/u-boot-sunxi u-boot "fork",
so that we can focus all u-boot work upstream.
I assume this means that u-boot-sunxi is going away (or at least only
being kept for historical reasons)
1. What happens to all the device memory info / configuration we have
in our "fork" of u-boot? I assume that'll all have to get ported
across at some point. (I don't have the code in front of me so if this
has already happened, then please ignore this)
Yeah that needs to be ported over my plan is to first get upstream u-boot
in a state where it can run unmodified android kernels, and then send a
mail to ask people to submit their board configs upstream. Upstream we
would like to have per board contact-persons (who actually own the board),
so blindly copy and pasting is undesirable. In the end we may end up just
copying the last few boards without having a contact person for them.
Post by Julian Calaby
2. I produced a script to detect duplicate dram_*.c files, will this
be useful upstream?
In the end we want to move to devicetree and have the dram timings in
devicetree. So I don't think having this in the tree is a good idea,
running it every now and then on the upstream code OTOH is a very good
idea :)

Regards,

Hans
Siarhei Siamashka
2014-10-16 10:14:37 UTC
Permalink
On Thu, 16 Oct 2014 10:23:42 +0200
Post by Hans de Goede
Hi,
Post by Julian Calaby
Hi Hans,
Post by Hans de Goede
Hi,
Post by Siarhei Siamashka
On Mon, 13 Oct 2014 13:10:28 +0200
Post by Hans de Goede
Hi,
Post by Siarhei Siamashka
When PLL5P is used as a parent clock for some of the peripherals,
the current code just selects some hardcoded divisors. This happens
to work, but only under assumption that the PLL5P clock speed is
somewhere between 360MHz and 480MHz (the typical DRAM clock speeds).
However with some tweaks for the DRAM parameters, it is possible to
http://lists.denx.de/pipermail/u-boot/2014-July/183981.html
And this introduces concerns about the hardcoded divisors in the
kernel, which may cause some peripherals to operate at abnormally
high clock speeds if the PLL5 clock speed is too fast (PLL5 is used
for clocking DRAM).
Moreover, it makes sense to avoid pre-dividing PLL5P and make it run
even faster than DRAM. This provides better granularity of the clock
speed selection for MBUS, G2D and everything else that is using PLL5P
as the parent clock. but running PLL5P faster means that the hardcoded
divisors become even more inappropriate.
This patch improves the clock divisors selection for G2D, ACE and
DEBE to insure that they can work correctly with any PLL5P clock
speed.
Can we please get this merged,
You know the rules. Every patch needs to be approved by at least one
person other than the submitter.
Thanks for the review. Pushed to stage/sunxi-3.4
Post by Hans de Goede
I'm working on getting the sunxi-3.4 kernels to work with upstream
u-boot, so that we can stop maintaining our own fork,
and this is necessary for this.
I hope we don't get in each other's way with this stuff.
I hope so too :)
I'm actually more or less done now, I've just send out 2 sunxi-3.4 patches
(which are quite similar to your 2, but for slightly different areas),
which completes my sunxi-3.4 work.
With these patches the 3.4 kernels will work on sun4i / sun5i with
an unmodified upstream u-boot. Still I've decided to revert your
PLL5 changes in upstream u-boot (for now) as I really want people to
be able to run unmodified android kernels on the upstream u-boot.
My main reason for this is that I want to take away any reasons
people may have to keep using the linux-sunxi/u-boot-sunxi u-boot "fork",
so that we can focus all u-boot work upstream.
I assume this means that u-boot-sunxi is going away (or at least only
being kept for historical reasons)
1. What happens to all the device memory info / configuration we have
in our "fork" of u-boot? I assume that'll all have to get ported
across at some point. (I don't have the code in front of me so if this
has already happened, then please ignore this)
Yeah that needs to be ported over my plan is to first get upstream u-boot
in a state where it can run unmodified android kernels,
Do you mean the kernels from the Allwinner SDK?
Post by Hans de Goede
and then send a
mail to ask people to submit their board configs upstream. Upstream we
would like to have per board contact-persons (who actually own the board),
so blindly copy and pasting is undesirable. In the end we may end up just
copying the last few boards without having a contact person for them.
Post by Julian Calaby
2. I produced a script to detect duplicate dram_*.c files, will this
be useful upstream?
In the end we want to move to devicetree and have the dram timings in
devicetree. So I don't think having this in the tree is a good idea,
running it every now and then on the upstream code OTOH is a very good
idea :)
Adding libfdt bloat to SPL? There is just no extra space to waste.
--
Best regards,
Siarhei Siamashka
Hans de Goede
2014-10-16 12:46:29 UTC
Permalink
Hi,
Post by Siarhei Siamashka
On Thu, 16 Oct 2014 10:23:42 +0200
Post by Hans de Goede
Hi,
Post by Julian Calaby
Hi Hans,
Post by Hans de Goede
Hi,
Post by Siarhei Siamashka
On Mon, 13 Oct 2014 13:10:28 +0200
Post by Hans de Goede
Hi,
Post by Siarhei Siamashka
When PLL5P is used as a parent clock for some of the peripherals,
the current code just selects some hardcoded divisors. This happens
to work, but only under assumption that the PLL5P clock speed is
somewhere between 360MHz and 480MHz (the typical DRAM clock speeds).
However with some tweaks for the DRAM parameters, it is possible to
http://lists.denx.de/pipermail/u-boot/2014-July/183981.html
And this introduces concerns about the hardcoded divisors in the
kernel, which may cause some peripherals to operate at abnormally
high clock speeds if the PLL5 clock speed is too fast (PLL5 is used
for clocking DRAM).
Moreover, it makes sense to avoid pre-dividing PLL5P and make it run
even faster than DRAM. This provides better granularity of the clock
speed selection for MBUS, G2D and everything else that is using PLL5P
as the parent clock. but running PLL5P faster means that the hardcoded
divisors become even more inappropriate.
This patch improves the clock divisors selection for G2D, ACE and
DEBE to insure that they can work correctly with any PLL5P clock
speed.
Can we please get this merged,
You know the rules. Every patch needs to be approved by at least one
person other than the submitter.
Thanks for the review. Pushed to stage/sunxi-3.4
Post by Hans de Goede
I'm working on getting the sunxi-3.4 kernels to work with upstream
u-boot, so that we can stop maintaining our own fork,
and this is necessary for this.
I hope we don't get in each other's way with this stuff.
I hope so too :)
I'm actually more or less done now, I've just send out 2 sunxi-3.4 patches
(which are quite similar to your 2, but for slightly different areas),
which completes my sunxi-3.4 work.
With these patches the 3.4 kernels will work on sun4i / sun5i with
an unmodified upstream u-boot. Still I've decided to revert your
PLL5 changes in upstream u-boot (for now) as I really want people to
be able to run unmodified android kernels on the upstream u-boot.
My main reason for this is that I want to take away any reasons
people may have to keep using the linux-sunxi/u-boot-sunxi u-boot "fork",
so that we can focus all u-boot work upstream.
I assume this means that u-boot-sunxi is going away (or at least only
being kept for historical reasons)
1. What happens to all the device memory info / configuration we have
in our "fork" of u-boot? I assume that'll all have to get ported
across at some point. (I don't have the code in front of me so if this
has already happened, then please ignore this)
Yeah that needs to be ported over my plan is to first get upstream u-boot
in a state where it can run unmodified android kernels,
Do you mean the kernels from the Allwinner SDK?
Preferably yes, not sure how doable that is, at a minimum we should support
the linux-sunxi-3.4 kernels.
Post by Siarhei Siamashka
Post by Hans de Goede
and then send a
mail to ask people to submit their board configs upstream. Upstream we
would like to have per board contact-persons (who actually own the board),
so blindly copy and pasting is undesirable. In the end we may end up just
copying the last few boards without having a contact person for them.
Post by Julian Calaby
2. I produced a script to detect duplicate dram_*.c files, will this
be useful upstream?
In the end we want to move to devicetree and have the dram timings in
devicetree. So I don't think having this in the tree is a good idea,
running it every now and then on the upstream code OTOH is a very good
idea :)
Adding libfdt bloat to SPL? There is just no extra space to waste.
This is something which still needs to be hashed out, one idea is to still
have per board spl binaries, and do the fdt parsing build-time.

Regards,

Hans

Arokux X
2014-10-15 22:47:44 UTC
Permalink
Post by Hans de Goede
With these patches the 3.4 kernels will work on sun4i / sun5i with
an unmodified upstream u-boot.
Just letting you know. Upstream u-boot won't load 3.4 kernel on sun7i
(cubietruck).

Best,
Arokux
Continue reading on narkive:
Loading...