* [PATCH v3 01/11] clk: Warn on failure to assign rate
2021-04-09 2:13 [PATCH v3 00/11] riscv: k210: Enable use of AI ram bank Sean Anderson
@ 2021-04-09 2:13 ` Sean Anderson
2021-04-09 2:13 ` [PATCH v3 02/11] clk: k210: Fix PLLs not being enabled Sean Anderson
` (9 subsequent siblings)
10 siblings, 0 replies; 18+ messages in thread
From: Sean Anderson @ 2021-04-09 2:13 UTC (permalink / raw)
To: u-boot
If the user/dev explicitly requests a clock be assigned a certain rate,
then we should warn them if we can't do it. This makes it clear if the
clock is running at the default rate.
Signed-off-by: Sean Anderson <seanga2@gmail.com>
---
Changes in v3:
- New
drivers/clk/clk-uclass.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
index 4ab3c402ed..53e7be764d 100644
--- a/drivers/clk/clk-uclass.c
+++ b/drivers/clk/clk-uclass.c
@@ -14,6 +14,7 @@
#include <errno.h>
#include <log.h>
#include <malloc.h>
+#include <dm/device_compat.h>
#include <dm/device-internal.h>
#include <dm/devres.h>
#include <dm/read.h>
@@ -309,8 +310,9 @@ static int clk_set_default_rates(struct udevice *dev, int stage)
ret = clk_get_by_indexed_prop(dev, "assigned-clocks",
index, &clk);
if (ret) {
- debug("%s: could not get assigned clock %d for %s\n",
- __func__, index, dev_read_name(dev));
+ dev_dbg(dev,
+ "could not get assigned clock %d (err = %d)\n",
+ index, ret);
continue;
}
@@ -332,8 +334,9 @@ static int clk_set_default_rates(struct udevice *dev, int stage)
ret = clk_set_rate(c, rates[index]);
if (ret < 0) {
- debug("%s: failed to set rate on clock index %d (%ld) for %s\n",
- __func__, index, clk.id, dev_read_name(dev));
+ dev_warn(dev,
+ "failed to set rate on clock index %d (%ld) (error = %d)\n",
+ index, clk.id, ret);
break;
}
}
--
2.31.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 02/11] clk: k210: Fix PLLs not being enabled
2021-04-09 2:13 [PATCH v3 00/11] riscv: k210: Enable use of AI ram bank Sean Anderson
2021-04-09 2:13 ` [PATCH v3 01/11] clk: Warn on failure to assign rate Sean Anderson
@ 2021-04-09 2:13 ` Sean Anderson
2021-04-09 2:45 ` Damien Le Moal
2021-04-09 2:13 ` [PATCH v3 03/11] clk: k210: Fix PLL enable always getting taken Sean Anderson
` (8 subsequent siblings)
10 siblings, 1 reply; 18+ messages in thread
From: Sean Anderson @ 2021-04-09 2:13 UTC (permalink / raw)
To: u-boot
After starting or setting the rate of a PLL, the enable bit must be set.
This fixes a bug where the AI ram would not be accessible, because it
requires PLL1 to be running.
Signed-off-by: Sean Anderson <seanga2@gmail.com>
---
(no changes since v1)
drivers/clk/kendryte/pll.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/clk/kendryte/pll.c b/drivers/clk/kendryte/pll.c
index ab6d75d585..f198920113 100644
--- a/drivers/clk/kendryte/pll.c
+++ b/drivers/clk/kendryte/pll.c
@@ -531,6 +531,7 @@ static int k210_pll_enable(struct clk *clk)
k210_pll_waitfor_lock(pll);
reg &= ~K210_PLL_BYPASS;
+ reg |= K210_PLL_EN;
writel(reg, pll->reg);
return 0;
@@ -550,6 +551,7 @@ static int k210_pll_disable(struct clk *clk)
writel(reg, pll->reg);
reg &= ~K210_PLL_PWRD;
+ reg &= ~K210_PLL_EN;
writel(reg, pll->reg);
return 0;
}
--
2.31.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 02/11] clk: k210: Fix PLLs not being enabled
2021-04-09 2:13 ` [PATCH v3 02/11] clk: k210: Fix PLLs not being enabled Sean Anderson
@ 2021-04-09 2:45 ` Damien Le Moal
0 siblings, 0 replies; 18+ messages in thread
From: Damien Le Moal @ 2021-04-09 2:45 UTC (permalink / raw)
To: u-boot
On 2021/04/09 11:13, Sean Anderson wrote:
> After starting or setting the rate of a PLL, the enable bit must be set.
>
> This fixes a bug where the AI ram would not be accessible, because it
> requires PLL1 to be running.
>
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> ---
>
> (no changes since v1)
>
> drivers/clk/kendryte/pll.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/clk/kendryte/pll.c b/drivers/clk/kendryte/pll.c
> index ab6d75d585..f198920113 100644
> --- a/drivers/clk/kendryte/pll.c
> +++ b/drivers/clk/kendryte/pll.c
> @@ -531,6 +531,7 @@ static int k210_pll_enable(struct clk *clk)
> k210_pll_waitfor_lock(pll);
>
> reg &= ~K210_PLL_BYPASS;
> + reg |= K210_PLL_EN;
> writel(reg, pll->reg);
>
> return 0;
> @@ -550,6 +551,7 @@ static int k210_pll_disable(struct clk *clk)
> writel(reg, pll->reg);
>
> reg &= ~K210_PLL_PWRD;
> + reg &= ~K210_PLL_EN;
> writel(reg, pll->reg);
> return 0;
> }
>
Looks good. That matches what the linux driver is doing.
Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3 03/11] clk: k210: Fix PLL enable always getting taken
2021-04-09 2:13 [PATCH v3 00/11] riscv: k210: Enable use of AI ram bank Sean Anderson
2021-04-09 2:13 ` [PATCH v3 01/11] clk: Warn on failure to assign rate Sean Anderson
2021-04-09 2:13 ` [PATCH v3 02/11] clk: k210: Fix PLLs not being enabled Sean Anderson
@ 2021-04-09 2:13 ` Sean Anderson
2021-04-09 2:13 ` [PATCH v3 04/11] clk: k210: Remove k210_register_pll Sean Anderson
` (7 subsequent siblings)
10 siblings, 0 replies; 18+ messages in thread
From: Sean Anderson @ 2021-04-09 2:13 UTC (permalink / raw)
To: u-boot
This conditional always evaluated as false, regardless of the value of reg.
Fix it so that it properly tests the bits in the PLL register. Also test
PLL_EN, now that we set it.
Reported-by: Damien Le Moal <Damien.LeMoal@wdc.com>
Signed-off-by: Sean Anderson <seanga2@gmail.com>
---
(no changes since v2)
Changes in v2:
- New
drivers/clk/kendryte/pll.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/kendryte/pll.c b/drivers/clk/kendryte/pll.c
index f198920113..d46fd0ebbf 100644
--- a/drivers/clk/kendryte/pll.c
+++ b/drivers/clk/kendryte/pll.c
@@ -512,7 +512,8 @@ static int k210_pll_enable(struct clk *clk)
struct k210_pll *pll = to_k210_pll(clk);
u32 reg = readl(pll->reg);
- if ((reg | K210_PLL_PWRD) && !(reg | K210_PLL_RESET))
+ if ((reg & K210_PLL_PWRD) && (reg & K210_PLL_EN) &&
+ !(reg & K210_PLL_RESET))
return 0;
reg |= K210_PLL_PWRD;
--
2.31.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 04/11] clk: k210: Remove k210_register_pll
2021-04-09 2:13 [PATCH v3 00/11] riscv: k210: Enable use of AI ram bank Sean Anderson
` (2 preceding siblings ...)
2021-04-09 2:13 ` [PATCH v3 03/11] clk: k210: Fix PLL enable always getting taken Sean Anderson
@ 2021-04-09 2:13 ` Sean Anderson
2021-04-09 2:13 ` [PATCH v3 05/11] clk: k210: Move the clint clock to under aclk Sean Anderson
` (6 subsequent siblings)
10 siblings, 0 replies; 18+ messages in thread
From: Sean Anderson @ 2021-04-09 2:13 UTC (permalink / raw)
To: u-boot
This simplifies the PLL creation process, since we don't have to pass all
the parameters individually.
Signed-off-by: Sean Anderson <seanga2@gmail.com>
---
(no changes since v2)
Changes in v2:
- New
drivers/clk/kendryte/clk.c | 10 +++-------
drivers/clk/kendryte/pll.c | 21 ---------------------
include/kendryte/pll.h | 4 ----
3 files changed, 3 insertions(+), 32 deletions(-)
diff --git a/drivers/clk/kendryte/clk.c b/drivers/clk/kendryte/clk.c
index 3b674a998e..ab86533bb4 100644
--- a/drivers/clk/kendryte/clk.c
+++ b/drivers/clk/kendryte/clk.c
@@ -528,14 +528,10 @@ static int k210_clk_probe(struct udevice *dev)
return -ENOMEM;
}
- {
- const struct k210_pll_params *params = &k210_plls[1];
-
+ pll = k210_create_pll(&k210_plls[1], base);
+ if (pll)
clk_dm(K210_CLK_PLL1,
- k210_register_pll("pll1", in0, base + params->off,
- base + params->lock_off, params->shift,
- params->width));
- }
+ k210_register_pll_struct("pll1", in0, pll));
/* PLL2 is muxed, so set up a composite clock */
mux = k210_create_mux(&k210_muxes[MUXIFY(K210_CLK_PLL2)], base);
diff --git a/drivers/clk/kendryte/pll.c b/drivers/clk/kendryte/pll.c
index d46fd0ebbf..184f37aaf2 100644
--- a/drivers/clk/kendryte/pll.c
+++ b/drivers/clk/kendryte/pll.c
@@ -578,27 +578,6 @@ struct clk *k210_register_pll_struct(const char *name, const char *parent_name,
return clk;
}
-struct clk *k210_register_pll(const char *name, const char *parent_name,
- void __iomem *reg, void __iomem *lock, u8 shift,
- u8 width)
-{
- struct clk *clk;
- struct k210_pll *pll;
-
- pll = kzalloc(sizeof(*pll), GFP_KERNEL);
- if (!pll)
- return ERR_PTR(-ENOMEM);
- pll->reg = reg;
- pll->lock = lock;
- pll->shift = shift;
- pll->width = width;
-
- clk = k210_register_pll_struct(name, parent_name, pll);
- if (IS_ERR(clk))
- kfree(pll);
- return clk;
-}
-
U_BOOT_DRIVER(k210_pll) = {
.name = CLK_K210_PLL,
.id = UCLASS_CLK,
diff --git a/include/kendryte/pll.h b/include/kendryte/pll.h
index 55a40b9c97..95b8494f40 100644
--- a/include/kendryte/pll.h
+++ b/include/kendryte/pll.h
@@ -55,8 +55,4 @@ extern const struct clk_ops k210_pll_ops;
struct clk *k210_register_pll_struct(const char *name, const char *parent_name,
struct k210_pll *pll);
-struct clk *k210_register_pll(const char *name, const char *parent_name,
- void __iomem *reg, void __iomem *lock, u8 shift,
- u8 width);
-
#endif /* K210_PLL_H */
--
2.31.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 05/11] clk: k210: Move the clint clock to under aclk
2021-04-09 2:13 [PATCH v3 00/11] riscv: k210: Enable use of AI ram bank Sean Anderson
` (3 preceding siblings ...)
2021-04-09 2:13 ` [PATCH v3 04/11] clk: k210: Remove k210_register_pll Sean Anderson
@ 2021-04-09 2:13 ` Sean Anderson
2021-04-09 2:54 ` Damien Le Moal
2021-04-09 2:13 ` [PATCH v3 06/11] clk: Add support for the k210 clock driver pre-relocation Sean Anderson
` (5 subsequent siblings)
10 siblings, 1 reply; 18+ messages in thread
From: Sean Anderson @ 2021-04-09 2:13 UTC (permalink / raw)
To: u-boot
No other (real) clocks have the cpu clock as their parent; instead they are
children of aclk. Move the clint clock under aclk to match them.
Signed-off-by: Sean Anderson <seanga2@gmail.com>
---
(no changes since v2)
Changes in v2:
- New
drivers/clk/kendryte/clk.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clk/kendryte/clk.c b/drivers/clk/kendryte/clk.c
index ab86533bb4..5091f07eb0 100644
--- a/drivers/clk/kendryte/clk.c
+++ b/drivers/clk/kendryte/clk.c
@@ -643,7 +643,7 @@ static int k210_clk_probe(struct udevice *dev)
/* The MTIME register in CLINT runs at one 50th the CPU clock speed */
clk_dm(K210_CLK_CLINT,
- clk_register_fixed_factor(NULL, "clint", "cpu", 0, 1, 50));
+ clk_register_fixed_factor(NULL, "clint", "aclk", 0, 1, 50));
return 0;
}
--
2.31.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 05/11] clk: k210: Move the clint clock to under aclk
2021-04-09 2:13 ` [PATCH v3 05/11] clk: k210: Move the clint clock to under aclk Sean Anderson
@ 2021-04-09 2:54 ` Damien Le Moal
2021-04-09 2:57 ` Sean Anderson
2021-04-09 2:58 ` Damien Le Moal
0 siblings, 2 replies; 18+ messages in thread
From: Damien Le Moal @ 2021-04-09 2:54 UTC (permalink / raw)
To: u-boot
On 2021/04/09 11:13, Sean Anderson wrote:
> No other (real) clocks have the cpu clock as their parent; instead they are
> children of aclk. Move the clint clock under aclk to match them.
>
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> ---
>
> (no changes since v2)
>
> Changes in v2:
> - New
>
> drivers/clk/kendryte/clk.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/clk/kendryte/clk.c b/drivers/clk/kendryte/clk.c
> index ab86533bb4..5091f07eb0 100644
> --- a/drivers/clk/kendryte/clk.c
> +++ b/drivers/clk/kendryte/clk.c
> @@ -643,7 +643,7 @@ static int k210_clk_probe(struct udevice *dev)
>
> /* The MTIME register in CLINT runs at one 50th the CPU clock speed */
> clk_dm(K210_CLK_CLINT,
> - clk_register_fixed_factor(NULL, "clint", "cpu", 0, 1, 50));
> + clk_register_fixed_factor(NULL, "clint", "aclk", 0, 1, 50));
>
> return 0;
> }
>
Looks good, but representing this clock is in fact useless, at least in Linux,
since the clint driver does not use it directly and derives its rate from
riscv_timebase which is set from the timebase-frequency DT property.
Not sure how u-boot handles that though. Since your code allows changing the
PLLs frequency, the timebase-frequency property may end up being buggy if it is
not changed too.
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3 05/11] clk: k210: Move the clint clock to under aclk
2021-04-09 2:54 ` Damien Le Moal
@ 2021-04-09 2:57 ` Sean Anderson
2021-04-09 3:00 ` Damien Le Moal
2021-04-09 2:58 ` Damien Le Moal
1 sibling, 1 reply; 18+ messages in thread
From: Sean Anderson @ 2021-04-09 2:57 UTC (permalink / raw)
To: u-boot
On 4/8/21 10:54 PM, Damien Le Moal wrote:
> On 2021/04/09 11:13, Sean Anderson wrote:
>> No other (real) clocks have the cpu clock as their parent; instead they are
>> children of aclk. Move the clint clock under aclk to match them.
>>
>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
>> ---
>>
>> (no changes since v2)
>>
>> Changes in v2:
>> - New
>>
>> drivers/clk/kendryte/clk.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/kendryte/clk.c b/drivers/clk/kendryte/clk.c
>> index ab86533bb4..5091f07eb0 100644
>> --- a/drivers/clk/kendryte/clk.c
>> +++ b/drivers/clk/kendryte/clk.c
>> @@ -643,7 +643,7 @@ static int k210_clk_probe(struct udevice *dev)
>>
>> /* The MTIME register in CLINT runs at one 50th the CPU clock speed */
>> clk_dm(K210_CLK_CLINT,
>> - clk_register_fixed_factor(NULL, "clint", "cpu", 0, 1, 50));
>> + clk_register_fixed_factor(NULL, "clint", "aclk", 0, 1, 50));
>>
>> return 0;
>> }
>>
>
> Looks good, but representing this clock is in fact useless, at least in Linux,
> since the clint driver does not use it directly and derives its rate from
> riscv_timebase which is set from the timebase-frequency DT property.
>
> Not sure how u-boot handles that though. Since your code allows changing the
> PLLs frequency, the timebase-frequency property may end up being buggy if it is
> not changed too.
>
U-Boot uses the clocks property since 47d7e3b5eb ("riscv: Move timer
portions of SiFive CLINT to drivers/timer") :)
--Sean
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3 05/11] clk: k210: Move the clint clock to under aclk
2021-04-09 2:57 ` Sean Anderson
@ 2021-04-09 3:00 ` Damien Le Moal
0 siblings, 0 replies; 18+ messages in thread
From: Damien Le Moal @ 2021-04-09 3:00 UTC (permalink / raw)
To: u-boot
On 2021/04/09 11:58, Sean Anderson wrote:
>
> On 4/8/21 10:54 PM, Damien Le Moal wrote:
>> On 2021/04/09 11:13, Sean Anderson wrote:
>>> No other (real) clocks have the cpu clock as their parent; instead they are
>>> children of aclk. Move the clint clock under aclk to match them.
>>>
>>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
>>> ---
>>>
>>> (no changes since v2)
>>>
>>> Changes in v2:
>>> - New
>>>
>>> drivers/clk/kendryte/clk.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/clk/kendryte/clk.c b/drivers/clk/kendryte/clk.c
>>> index ab86533bb4..5091f07eb0 100644
>>> --- a/drivers/clk/kendryte/clk.c
>>> +++ b/drivers/clk/kendryte/clk.c
>>> @@ -643,7 +643,7 @@ static int k210_clk_probe(struct udevice *dev)
>>>
>>> /* The MTIME register in CLINT runs at one 50th the CPU clock speed */
>>> clk_dm(K210_CLK_CLINT,
>>> - clk_register_fixed_factor(NULL, "clint", "cpu", 0, 1, 50));
>>> + clk_register_fixed_factor(NULL, "clint", "aclk", 0, 1, 50));
>>>
>>> return 0;
>>> }
>>>
>>
>> Looks good, but representing this clock is in fact useless, at least in Linux,
>> since the clint driver does not use it directly and derives its rate from
>> riscv_timebase which is set from the timebase-frequency DT property.
>>
>> Not sure how u-boot handles that though. Since your code allows changing the
>> PLLs frequency, the timebase-frequency property may end up being buggy if it is
>> not changed too.
>>
>
> U-Boot uses the clocks property since 47d7e3b5eb ("riscv: Move timer
> portions of SiFive CLINT to drivers/timer") :)
I remember that adding the clock property to Linux DT gave warning with make
dtb_check. Hence I removed it. driver/clocksoure/timer-riscv.c also make the
timebase-frequency DT property mandatory. Time for some more patching on Linux
side I guess :)
>
> --Sean
>
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3 05/11] clk: k210: Move the clint clock to under aclk
2021-04-09 2:54 ` Damien Le Moal
2021-04-09 2:57 ` Sean Anderson
@ 2021-04-09 2:58 ` Damien Le Moal
2021-04-09 3:09 ` Sean Anderson
1 sibling, 1 reply; 18+ messages in thread
From: Damien Le Moal @ 2021-04-09 2:58 UTC (permalink / raw)
To: u-boot
On 2021/04/09 11:54, Damien Le Moal wrote:
> On 2021/04/09 11:13, Sean Anderson wrote:
>> No other (real) clocks have the cpu clock as their parent; instead they are
>> children of aclk. Move the clint clock under aclk to match them.
>>
>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
>> ---
>>
>> (no changes since v2)
>>
>> Changes in v2:
>> - New
>>
>> drivers/clk/kendryte/clk.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/kendryte/clk.c b/drivers/clk/kendryte/clk.c
>> index ab86533bb4..5091f07eb0 100644
>> --- a/drivers/clk/kendryte/clk.c
>> +++ b/drivers/clk/kendryte/clk.c
>> @@ -643,7 +643,7 @@ static int k210_clk_probe(struct udevice *dev)
>>
>> /* The MTIME register in CLINT runs at one 50th the CPU clock speed */
>> clk_dm(K210_CLK_CLINT,
>> - clk_register_fixed_factor(NULL, "clint", "cpu", 0, 1, 50));
>> + clk_register_fixed_factor(NULL, "clint", "aclk", 0, 1, 50));
>>
>> return 0;
>> }
>>
>
> Looks good, but representing this clock is in fact useless, at least in Linux,
> since the clint driver does not use it directly and derives its rate from
> riscv_timebase which is set from the timebase-frequency DT property.
>
> Not sure how u-boot handles that though. Since your code allows changing the
> PLLs frequency, the timebase-frequency property may end up being buggy if it is
> not changed too.
Actually, thinking about this some more, the clint DT node should have an
optional clock entry and use that clock rate to set riscv_timebase if the clock
entry is present. riscv_timebase can be set using timebase-frequency DT property
if the clock is not set.
>
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3 05/11] clk: k210: Move the clint clock to under aclk
2021-04-09 2:58 ` Damien Le Moal
@ 2021-04-09 3:09 ` Sean Anderson
0 siblings, 0 replies; 18+ messages in thread
From: Sean Anderson @ 2021-04-09 3:09 UTC (permalink / raw)
To: u-boot
On 4/8/21 10:58 PM, Damien Le Moal wrote:
> On 2021/04/09 11:54, Damien Le Moal wrote:
>> On 2021/04/09 11:13, Sean Anderson wrote:
>>> No other (real) clocks have the cpu clock as their parent; instead they are
>>> children of aclk. Move the clint clock under aclk to match them.
>>>
>>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
>>> ---
>>>
>>> (no changes since v2)
>>>
>>> Changes in v2:
>>> - New
>>>
>>> drivers/clk/kendryte/clk.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/clk/kendryte/clk.c b/drivers/clk/kendryte/clk.c
>>> index ab86533bb4..5091f07eb0 100644
>>> --- a/drivers/clk/kendryte/clk.c
>>> +++ b/drivers/clk/kendryte/clk.c
>>> @@ -643,7 +643,7 @@ static int k210_clk_probe(struct udevice *dev)
>>>
>>> /* The MTIME register in CLINT runs at one 50th the CPU clock speed */
>>> clk_dm(K210_CLK_CLINT,
>>> - clk_register_fixed_factor(NULL, "clint", "cpu", 0, 1, 50));
>>> + clk_register_fixed_factor(NULL, "clint", "aclk", 0, 1, 50));
>>>
>>> return 0;
>>> }
>>>
>>
>> Looks good, but representing this clock is in fact useless, at least in Linux,
>> since the clint driver does not use it directly and derives its rate from
>> riscv_timebase which is set from the timebase-frequency DT property.
>>
>> Not sure how u-boot handles that though. Since your code allows changing the
>> PLLs frequency, the timebase-frequency property may end up being buggy if it is
>> not changed too.
>
> Actually, thinking about this some more, the clint DT node should have an
> optional clock entry and use that clock rate to set riscv_timebase if the clock
> entry is present. riscv_timebase can be set using timebase-frequency DT property
> if the clock is not set.
This is how it is done in U-Boot. See timer_pre_probe and
timer_timebase_fallback for the mechanism.
--Sean
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3 06/11] clk: Add support for the k210 clock driver pre-relocation
2021-04-09 2:13 [PATCH v3 00/11] riscv: k210: Enable use of AI ram bank Sean Anderson
` (4 preceding siblings ...)
2021-04-09 2:13 ` [PATCH v3 05/11] clk: k210: Move the clint clock to under aclk Sean Anderson
@ 2021-04-09 2:13 ` Sean Anderson
2021-04-09 2:13 ` [PATCH v3 07/11] riscv: Enable some devices pre-relocation Sean Anderson
` (4 subsequent siblings)
10 siblings, 0 replies; 18+ messages in thread
From: Sean Anderson @ 2021-04-09 2:13 UTC (permalink / raw)
To: u-boot
Variables which had previously been stored in .bss are moved to .data. In
addition, probed needs to be reset when the clock driver is re-bound
post-relocation.
Signed-off-by: Sean Anderson <seanga2@gmail.com>
---
(no changes since v1)
drivers/clk/kendryte/clk.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/clk/kendryte/clk.c b/drivers/clk/kendryte/clk.c
index 5091f07eb0..2d6ac03693 100644
--- a/drivers/clk/kendryte/clk.c
+++ b/drivers/clk/kendryte/clk.c
@@ -347,9 +347,7 @@ static const struct k210_comp_params k210_comps[] = {
#undef COMP_NOMUX_ID
#undef COMP_LIST
-static struct clk *k210_bypass_children = {
- NULL,
-};
+static struct clk *k210_bypass_children __section(.data);
/* Helper functions to create sub-clocks */
static struct clk_mux *k210_create_mux(const struct k210_mux_params *params,
@@ -475,7 +473,14 @@ cleanup_mux:
return comp;
}
-static bool probed;
+static bool __section(.data) probed;
+
+/* reset probed so we will probe again post-relocation */
+static int k210_clk_bind(struct udevice *dev)
+{
+ probed = false;
+ return 0;
+}
static int k210_clk_probe(struct udevice *dev)
{
@@ -658,5 +663,6 @@ U_BOOT_DRIVER(k210_clk) = {
.id = UCLASS_CLK,
.of_match = k210_clk_ids,
.ops = &k210_clk_ops,
+ .bind = k210_clk_bind,
.probe = k210_clk_probe,
};
--
2.31.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 07/11] riscv: Enable some devices pre-relocation
2021-04-09 2:13 [PATCH v3 00/11] riscv: k210: Enable use of AI ram bank Sean Anderson
` (5 preceding siblings ...)
2021-04-09 2:13 ` [PATCH v3 06/11] clk: Add support for the k210 clock driver pre-relocation Sean Anderson
@ 2021-04-09 2:13 ` Sean Anderson
2021-04-09 2:13 ` [PATCH v3 08/11] riscv: Enable AI ram on K210 Sean Anderson
` (3 subsequent siblings)
10 siblings, 0 replies; 18+ messages in thread
From: Sean Anderson @ 2021-04-09 2:13 UTC (permalink / raw)
To: u-boot
These devices are necessary for the clock driver, which is required by the
sram driver, to run pre-relocation.
Signed-off-by: Sean Anderson <seanga2@gmail.com>
---
(no changes since v1)
arch/riscv/dts/k210.dtsi | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/riscv/dts/k210.dtsi b/arch/riscv/dts/k210.dtsi
index 0b79a29600..a735bebf2c 100644
--- a/arch/riscv/dts/k210.dtsi
+++ b/arch/riscv/dts/k210.dtsi
@@ -91,6 +91,7 @@
<&sysclk K210_CLK_SRAM1>,
<&sysclk K210_CLK_PLL1>;
clock-names = "sram0", "sram1", "airam";
+ u-boot,dm-pre-reloc;
};
reserved-memory {
@@ -109,6 +110,7 @@
compatible = "fixed-clock";
#clock-cells = <0>;
clock-frequency = <26000000>;
+ u-boot,dm-pre-reloc;
};
};
@@ -505,11 +507,13 @@
"syscon", "simple-mfd";
reg = <0x50440000 0x100>;
reg-io-width = <4>;
+ u-boot,dm-pre-reloc;
sysclk: clock-controller {
#clock-cells = <1>;
compatible = "kendryte,k210-clk";
clocks = <&in0>;
+ u-boot,dm-pre-reloc;
};
sysrst: reset-controller {
--
2.31.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 08/11] riscv: Enable AI ram on K210
2021-04-09 2:13 [PATCH v3 00/11] riscv: k210: Enable use of AI ram bank Sean Anderson
` (6 preceding siblings ...)
2021-04-09 2:13 ` [PATCH v3 07/11] riscv: Enable some devices pre-relocation Sean Anderson
@ 2021-04-09 2:13 ` Sean Anderson
2021-04-09 2:13 ` [PATCH v3 09/11] riscv: k210: Rename airam to aisram Sean Anderson
` (2 subsequent siblings)
10 siblings, 0 replies; 18+ messages in thread
From: Sean Anderson @ 2021-04-09 2:13 UTC (permalink / raw)
To: u-boot
We just need to initialize all the clocks pre-reloc. The clock driver
creates a bunch of devices, so we need to increase the pre-reloc malloc
arena.
Signed-off-by: Sean Anderson <seanga2@gmail.com>
---
(no changes since v1)
board/sipeed/maix/maix.c | 12 +++++++++++-
configs/sipeed_maix_bitm_defconfig | 2 ++
include/configs/sipeed-maix.h | 3 +--
3 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/board/sipeed/maix/maix.c b/board/sipeed/maix/maix.c
index cbcb23cf5c..6e582911f8 100644
--- a/board/sipeed/maix/maix.c
+++ b/board/sipeed/maix/maix.c
@@ -14,7 +14,7 @@ phys_size_t get_effective_memsize(void)
return CONFIG_SYS_SDRAM_SIZE;
}
-int board_init(void)
+static int sram_init(void)
{
int ret, i;
const char * const banks[] = { "sram0", "sram1", "airam" };
@@ -39,3 +39,13 @@ int board_init(void)
return 0;
}
+
+int board_early_init_f(void)
+{
+ return sram_init();
+}
+
+int board_init(void)
+{
+ return 0;
+}
diff --git a/configs/sipeed_maix_bitm_defconfig b/configs/sipeed_maix_bitm_defconfig
index 210848cccf..bd877cd055 100644
--- a/configs/sipeed_maix_bitm_defconfig
+++ b/configs/sipeed_maix_bitm_defconfig
@@ -1,4 +1,5 @@
CONFIG_RISCV=y
+CONFIG_SYS_MALLOC_F_LEN=0x10000
CONFIG_ENV_SIZE=0x1000
CONFIG_ENV_OFFSET=0xfff000
CONFIG_ENV_SECT_SIZE=0x1000
@@ -7,6 +8,7 @@ CONFIG_ARCH_RV64I=y
CONFIG_STACK_SIZE=0x100000
CONFIG_USE_BOOTCOMMAND=y
CONFIG_BOOTCOMMAND="run k210_bootcmd"
+CONFIG_BOARD_EARLY_INIT_F=y
CONFIG_HUSH_PARSER=y
CONFIG_MTDIDS_DEFAULT="nor0=spi3:0"
CONFIG_MTDPARTS_DEFAULT="nor0:1M(u-boot),0x1000 at 0xfff000(env)"
diff --git a/include/configs/sipeed-maix.h b/include/configs/sipeed-maix.h
index 4c1ff98ec6..0fbe8a5905 100644
--- a/include/configs/sipeed-maix.h
+++ b/include/configs/sipeed-maix.h
@@ -15,8 +15,7 @@
#define CONFIG_SYS_CACHELINE_SIZE 64
#define CONFIG_SYS_SDRAM_BASE 0x80000000
-/* Don't relocate into AI ram since it isn't set up yet */
-#define CONFIG_SYS_SDRAM_SIZE (SZ_4M + SZ_2M)
+#define CONFIG_SYS_SDRAM_SIZE SZ_8M
#ifndef CONFIG_EXTRA_ENV_SETTINGS
#define CONFIG_EXTRA_ENV_SETTINGS \
--
2.31.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 09/11] riscv: k210: Rename airam to aisram
2021-04-09 2:13 [PATCH v3 00/11] riscv: k210: Enable use of AI ram bank Sean Anderson
` (7 preceding siblings ...)
2021-04-09 2:13 ` [PATCH v3 08/11] riscv: Enable AI ram on K210 Sean Anderson
@ 2021-04-09 2:13 ` Sean Anderson
2021-04-09 2:13 ` [PATCH v3 10/11] riscv: k210: Use AI as the parent clock of aisram, not PLL1 Sean Anderson
2021-04-09 2:13 ` [PATCH v3 11/11] riscv: Don't reserve AI ram in k210 dts Sean Anderson
10 siblings, 0 replies; 18+ messages in thread
From: Sean Anderson @ 2021-04-09 2:13 UTC (permalink / raw)
To: u-boot
This is more consistent with the naming of other ram banks, and matches
what Linux is doing.
Reported-by: Damien Le Moal <Damien.LeMoal@wdc.com>
Signed-off-by: Sean Anderson <seanga2@gmail.com>
---
(no changes since v2)
Changes in v2:
- New
arch/riscv/dts/k210.dtsi | 4 ++--
board/sipeed/maix/maix.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/riscv/dts/k210.dtsi b/arch/riscv/dts/k210.dtsi
index a735bebf2c..2032f1e5c2 100644
--- a/arch/riscv/dts/k210.dtsi
+++ b/arch/riscv/dts/k210.dtsi
@@ -86,11 +86,11 @@
reg = <0x80000000 0x400000>,
<0x80400000 0x200000>,
<0x80600000 0x200000>;
- reg-names = "sram0", "sram1", "airam";
+ reg-names = "sram0", "sram1", "aisram";
clocks = <&sysclk K210_CLK_SRAM0>,
<&sysclk K210_CLK_SRAM1>,
<&sysclk K210_CLK_PLL1>;
- clock-names = "sram0", "sram1", "airam";
+ clock-names = "sram0", "sram1", "aisram";
u-boot,dm-pre-reloc;
};
diff --git a/board/sipeed/maix/maix.c b/board/sipeed/maix/maix.c
index 6e582911f8..52e4fee2f0 100644
--- a/board/sipeed/maix/maix.c
+++ b/board/sipeed/maix/maix.c
@@ -17,7 +17,7 @@ phys_size_t get_effective_memsize(void)
static int sram_init(void)
{
int ret, i;
- const char * const banks[] = { "sram0", "sram1", "airam" };
+ const char * const banks[] = { "sram0", "sram1", "aisram" };
ofnode memory;
struct clk clk;
--
2.31.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 10/11] riscv: k210: Use AI as the parent clock of aisram, not PLL1
2021-04-09 2:13 [PATCH v3 00/11] riscv: k210: Enable use of AI ram bank Sean Anderson
` (8 preceding siblings ...)
2021-04-09 2:13 ` [PATCH v3 09/11] riscv: k210: Rename airam to aisram Sean Anderson
@ 2021-04-09 2:13 ` Sean Anderson
2021-04-09 2:13 ` [PATCH v3 11/11] riscv: Don't reserve AI ram in k210 dts Sean Anderson
10 siblings, 0 replies; 18+ messages in thread
From: Sean Anderson @ 2021-04-09 2:13 UTC (permalink / raw)
To: u-boot
Testing showed that disabling AI while leaving PLL1 enabled disabled the
aisram. This suggests that AI is a more appropriate clock for that ram
bank.
Signed-off-by: Sean Anderson <seanga2@gmail.com>
---
(no changes since v2)
Changes in v2:
- New
arch/riscv/dts/k210.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/riscv/dts/k210.dtsi b/arch/riscv/dts/k210.dtsi
index 2032f1e5c2..75e101530b 100644
--- a/arch/riscv/dts/k210.dtsi
+++ b/arch/riscv/dts/k210.dtsi
@@ -89,7 +89,7 @@
reg-names = "sram0", "sram1", "aisram";
clocks = <&sysclk K210_CLK_SRAM0>,
<&sysclk K210_CLK_SRAM1>,
- <&sysclk K210_CLK_PLL1>;
+ <&sysclk K210_CLK_AI>;
clock-names = "sram0", "sram1", "aisram";
u-boot,dm-pre-reloc;
};
--
2.31.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 11/11] riscv: Don't reserve AI ram in k210 dts
2021-04-09 2:13 [PATCH v3 00/11] riscv: k210: Enable use of AI ram bank Sean Anderson
` (9 preceding siblings ...)
2021-04-09 2:13 ` [PATCH v3 10/11] riscv: k210: Use AI as the parent clock of aisram, not PLL1 Sean Anderson
@ 2021-04-09 2:13 ` Sean Anderson
10 siblings, 0 replies; 18+ messages in thread
From: Sean Anderson @ 2021-04-09 2:13 UTC (permalink / raw)
To: u-boot
It is no longer necessary to disallow ai ram, since it is enabled by the
sram driver.
Signed-off-by: Sean Anderson <seanga2@gmail.com>
---
(no changes since v1)
arch/riscv/dts/k210.dtsi | 12 ------------
1 file changed, 12 deletions(-)
diff --git a/arch/riscv/dts/k210.dtsi b/arch/riscv/dts/k210.dtsi
index 75e101530b..2492af8038 100644
--- a/arch/riscv/dts/k210.dtsi
+++ b/arch/riscv/dts/k210.dtsi
@@ -94,17 +94,6 @@
u-boot,dm-pre-reloc;
};
- reserved-memory {
- #address-cells = <1>;
- #size-cells = <1>;
- ranges;
-
- ai_reserved: ai at 80600000 {
- reg = <0x80600000 0x200000>;
- reusable;
- };
- };
-
clocks {
in0: osc {
compatible = "fixed-clock";
@@ -179,7 +168,6 @@
reg = <0x40800000 0xc00000>;
interrupts = <25>;
clocks = <&sysclk K210_CLK_AI>;
- memory-region = <&ai_reserved>;
status = "disabled";
};
--
2.31.0
^ permalink raw reply related [flat|nested] 18+ messages in thread