[PATCH] timer: starfive: Add Starfive timer support
Add timer driver in Starfive SoC. It is an timer that outside of CPU core and inside Starfive SoC.
Signed-off-by: Kuan Lim Lee kuanlim.lee@starfivetech.com Reviewed-by: Wei Liang Lim weiliang.lim@starfivetech.com --- drivers/timer/Kconfig | 7 +++ drivers/timer/Makefile | 1 + drivers/timer/starfive-timer.c | 94 ++++++++++++++++++++++++++++++++++ 3 files changed, 102 insertions(+) create mode 100644 drivers/timer/starfive-timer.c
diff --git a/drivers/timer/Kconfig b/drivers/timer/Kconfig index 915b2af160..a98be9dfae 100644 --- a/drivers/timer/Kconfig +++ b/drivers/timer/Kconfig @@ -326,4 +326,11 @@ config XILINX_TIMER Select this to enable support for the timer found on any Xilinx boards (axi timer).
+config STARFIVE_TIMER + bool "Starfive timer support" + depends on TIMER + help + Select this to enable support for the timer found on + Starfive SoC. + endmenu diff --git a/drivers/timer/Makefile b/drivers/timer/Makefile index 1ca74805fd..1ef814970b 100644 --- a/drivers/timer/Makefile +++ b/drivers/timer/Makefile @@ -34,3 +34,4 @@ obj-$(CONFIG_MTK_TIMER) += mtk_timer.o obj-$(CONFIG_MCHP_PIT64B_TIMER) += mchp-pit64b-timer.o obj-$(CONFIG_IMX_GPT_TIMER) += imx-gpt-timer.o obj-$(CONFIG_XILINX_TIMER) += xilinx-timer.o +obj-$(CONFIG_STARFIVE_TIMER) += starfive-timer.o diff --git a/drivers/timer/starfive-timer.c b/drivers/timer/starfive-timer.c new file mode 100644 index 0000000000..816402fdbf --- /dev/null +++ b/drivers/timer/starfive-timer.c @@ -0,0 +1,94 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright 2022 StarFive, Inc. All rights reserved. + * Author: Lee Kuan Lim kuanlim.lee@starfivetech.com + */ + +#include <common.h> +#include <clk.h> +#include <dm.h> +#include <time.h> +#include <timer.h> +#include <asm/io.h> +#include <dm/device-internal.h> +#include <linux/err.h> + +#define STF_TIMER_INT_STATUS 0x00 +#define STF_TIMER_CTL 0x04 +#define STF_TIMER_LOAD 0x08 +#define STF_TIMER_ENABLE 0x10 +#define STF_TIMER_RELOAD 0x14 +#define STF_TIMER_VALUE 0x18 +#define STF_TIMER_INT_CLR 0x20 +#define STF_TIMER_INT_MASK 0x24 + +struct starfive_timer_priv { + void __iomem *base; + u32 timer_size; +}; + +static u64 notrace starfive_get_count(struct udevice *dev) +{ + struct starfive_timer_priv *priv = dev_get_priv(dev); + + /* Read decrement timer value and convert to increment value */ + return priv->timer_size - readl(priv->base + STF_TIMER_VALUE); +} + +static const struct timer_ops starfive_ops = { + .get_count = starfive_get_count, +}; + +static int starfive_probe(struct udevice *dev) +{ + struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev); + struct starfive_timer_priv *priv = dev_get_priv(dev); + int timer_channel; + struct clk clk; + int ret; + + priv->base = dev_read_addr_ptr(dev); + if (IS_ERR(priv->base)) + return PTR_ERR(priv->base); + + timer_channel = dev_read_u32_default(dev, "channel", 0); + priv->base = priv->base + (0x40 * timer_channel); + + /* Get clock rate from channel selectecd*/ + ret = clk_get_by_index(dev, timer_channel, &clk); + if (ret) + return ret; + + ret = clk_enable(&clk); + if (ret) + return ret; + uc_priv->clock_rate = clk_get_rate(&clk); + + /* Initiate timer, channel 0 */ + /* Unmask Interrupt Mask */ + writel(0, priv->base + STF_TIMER_INT_MASK); + /* Single run mode Setting */ + if (dev_read_bool(dev, "single-run")) + writel(1, priv->base + STF_TIMER_CTL); + /* Set Reload value */ + priv->timer_size = dev_read_u32_default(dev, "timer-size", 0xffffffff); + writel(priv->timer_size, priv->base + STF_TIMER_LOAD); + /* Enable to start timer */ + writel(1, priv->base + STF_TIMER_ENABLE); + + return 0; +} + +static const struct udevice_id starfive_ids[] = { + { .compatible = "starfive,jh8100-timers" }, + { } +}; + +U_BOOT_DRIVER(jh8100_starfive_timer) = { + .name = "jh8100_starfive_timer", + .id = UCLASS_TIMER, + .of_match = starfive_ids, + .probe = starfive_probe, + .ops = &starfive_ops, + .priv_auto = sizeof(struct starfive_timer_priv), +};
Can anyone help review this patch please?
-----Original Message----- From: KuanLim.Lee kuanlim.lee@starfivetech.com Sent: Tuesday, September 19, 2023 3:31 PM To: u-boot@lists.denx.de Cc: WeiLiang Lim weiliang.lim@starfivetech.com; KuanLim.Lee kuanlim.lee@starfivetech.com Subject: [PATCH] timer: starfive: Add Starfive timer support
Add timer driver in Starfive SoC. It is an timer that outside of CPU core and inside Starfive SoC.
Signed-off-by: Kuan Lim Lee kuanlim.lee@starfivetech.com Reviewed-by: Wei Liang Lim weiliang.lim@starfivetech.com
drivers/timer/Kconfig | 7 +++ drivers/timer/Makefile | 1 + drivers/timer/starfive-timer.c | 94 ++++++++++++++++++++++++++++++++++ 3 files changed, 102 insertions(+) create mode 100644 drivers/timer/starfive-timer.c
diff --git a/drivers/timer/Kconfig b/drivers/timer/Kconfig index 915b2af160..a98be9dfae 100644 --- a/drivers/timer/Kconfig +++ b/drivers/timer/Kconfig @@ -326,4 +326,11 @@ config XILINX_TIMER Select this to enable support for the timer found on any Xilinx boards (axi timer).
+config STARFIVE_TIMER
- bool "Starfive timer support"
- depends on TIMER
- help
Select this to enable support for the timer found onStarfive SoC.endmenu diff --git a/drivers/timer/Makefile b/drivers/timer/Makefile index 1ca74805fd..1ef814970b 100644 --- a/drivers/timer/Makefile +++ b/drivers/timer/Makefile @@ -34,3 +34,4 @@ obj-$(CONFIG_MTK_TIMER) += mtk_timer.o obj-$(CONFIG_MCHP_PIT64B_TIMER) += mchp-pit64b-timer.o obj-$(CONFIG_IMX_GPT_TIMER) += imx-gpt-timer.o obj-$(CONFIG_XILINX_TIMER) += xilinx-timer.o +obj-$(CONFIG_STARFIVE_TIMER) += starfive-timer.o diff --git a/drivers/timer/starfive-timer.c b/drivers/timer/starfive-timer.c new file mode 100644 index 0000000000..816402fdbf --- /dev/null +++ b/drivers/timer/starfive-timer.c @@ -0,0 +1,94 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright 2022 StarFive, Inc. All rights reserved.
- Author: Lee Kuan Lim kuanlim.lee@starfivetech.com
- */
+#include <common.h> +#include <clk.h> +#include <dm.h> +#include <time.h> +#include <timer.h> +#include <asm/io.h> +#include <dm/device-internal.h> +#include <linux/err.h>
+#define STF_TIMER_INT_STATUS 0x00 +#define STF_TIMER_CTL 0x04 +#define STF_TIMER_LOAD 0x08 +#define STF_TIMER_ENABLE 0x10 +#define STF_TIMER_RELOAD 0x14 +#define STF_TIMER_VALUE 0x18 +#define STF_TIMER_INT_CLR 0x20 +#define STF_TIMER_INT_MASK 0x24
+struct starfive_timer_priv {
- void __iomem *base;
- u32 timer_size;
+};
+static u64 notrace starfive_get_count(struct udevice *dev) {
- struct starfive_timer_priv *priv = dev_get_priv(dev);
- /* Read decrement timer value and convert to increment value */
- return priv->timer_size - readl(priv->base + STF_TIMER_VALUE); }
+static const struct timer_ops starfive_ops = {
- .get_count = starfive_get_count,
+};
+static int starfive_probe(struct udevice *dev) {
- struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev);
- struct starfive_timer_priv *priv = dev_get_priv(dev);
- int timer_channel;
- struct clk clk;
- int ret;
- priv->base = dev_read_addr_ptr(dev);
- if (IS_ERR(priv->base))
return PTR_ERR(priv->base);- timer_channel = dev_read_u32_default(dev, "channel", 0);
- priv->base = priv->base + (0x40 * timer_channel);
- /* Get clock rate from channel selectecd*/
- ret = clk_get_by_index(dev, timer_channel, &clk);
- if (ret)
return ret;- ret = clk_enable(&clk);
- if (ret)
return ret;- uc_priv->clock_rate = clk_get_rate(&clk);
- /* Initiate timer, channel 0 */
- /* Unmask Interrupt Mask */
- writel(0, priv->base + STF_TIMER_INT_MASK);
- /* Single run mode Setting */
- if (dev_read_bool(dev, "single-run"))
writel(1, priv->base + STF_TIMER_CTL);- /* Set Reload value */
- priv->timer_size = dev_read_u32_default(dev, "timer-size", 0xffffffff);
- writel(priv->timer_size, priv->base + STF_TIMER_LOAD);
- /* Enable to start timer */
- writel(1, priv->base + STF_TIMER_ENABLE);
- return 0;
+}
+static const struct udevice_id starfive_ids[] = {
- { .compatible = "starfive,jh8100-timers" },
- { }
+};
+U_BOOT_DRIVER(jh8100_starfive_timer) = {
- .name = "jh8100_starfive_timer",
- .id = UCLASS_TIMER,
- .of_match = starfive_ids,
- .probe = starfive_probe,
- .ops = &starfive_ops,
- .priv_auto = sizeof(struct starfive_timer_priv),
+};
2.34.1
On Tue, 19 Sept 2023 at 06:08, Kuan Lim Lee kuanlim.lee@starfivetech.com wrote:
Add timer driver in Starfive SoC. It is an timer that outside of CPU core and inside Starfive SoC.
Signed-off-by: Kuan Lim Lee kuanlim.lee@starfivetech.com Reviewed-by: Wei Liang Lim weiliang.lim@starfivetech.com
drivers/timer/Kconfig | 7 +++ drivers/timer/Makefile | 1 + drivers/timer/starfive-timer.c | 94 ++++++++++++++++++++++++++++++++++ 3 files changed, 102 insertions(+) create mode 100644 drivers/timer/starfive-timer.c
Reviewed-by: Simon Glass sjg@chromium.org
nits below
diff --git a/drivers/timer/Kconfig b/drivers/timer/Kconfig index 915b2af160..a98be9dfae 100644 --- a/drivers/timer/Kconfig +++ b/drivers/timer/Kconfig @@ -326,4 +326,11 @@ config XILINX_TIMER Select this to enable support for the timer found on any Xilinx boards (axi timer).
+config STARFIVE_TIMER
bool "Starfive timer support"depends on TIMERhelpSelect this to enable support for the timer found onStarfive SoC.
What resolution is the timer? How is it clocked? Is there only one channel?
endmenu diff --git a/drivers/timer/Makefile b/drivers/timer/Makefile index 1ca74805fd..1ef814970b 100644 --- a/drivers/timer/Makefile +++ b/drivers/timer/Makefile @@ -34,3 +34,4 @@ obj-$(CONFIG_MTK_TIMER) += mtk_timer.o obj-$(CONFIG_MCHP_PIT64B_TIMER) += mchp-pit64b-timer.o obj-$(CONFIG_IMX_GPT_TIMER) += imx-gpt-timer.o obj-$(CONFIG_XILINX_TIMER) += xilinx-timer.o +obj-$(CONFIG_STARFIVE_TIMER) += starfive-timer.o diff --git a/drivers/timer/starfive-timer.c b/drivers/timer/starfive-timer.c new file mode 100644 index 0000000000..816402fdbf --- /dev/null +++ b/drivers/timer/starfive-timer.c @@ -0,0 +1,94 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright 2022 StarFive, Inc. All rights reserved.
- Author: Lee Kuan Lim kuanlim.lee@starfivetech.com
- */
+#include <common.h> +#include <clk.h> +#include <dm.h> +#include <time.h> +#include <timer.h> +#include <asm/io.h> +#include <dm/device-internal.h> +#include <linux/err.h>
+#define STF_TIMER_INT_STATUS 0x00 +#define STF_TIMER_CTL 0x04 +#define STF_TIMER_LOAD 0x08 +#define STF_TIMER_ENABLE 0x10 +#define STF_TIMER_RELOAD 0x14 +#define STF_TIMER_VALUE 0x18 +#define STF_TIMER_INT_CLR 0x20 +#define STF_TIMER_INT_MASK 0x24
+struct starfive_timer_priv {
void __iomem *base;u32 timer_size;+};
+static u64 notrace starfive_get_count(struct udevice *dev) +{
struct starfive_timer_priv *priv = dev_get_priv(dev);/* Read decrement timer value and convert to increment value */return priv->timer_size - readl(priv->base + STF_TIMER_VALUE);+}
As an enhancement, you could provide a timer_early_get_count() function and an easly setup, so you can use bootstage.
+static const struct timer_ops starfive_ops = {
.get_count = starfive_get_count,+};
+static int starfive_probe(struct udevice *dev) +{
struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev);struct starfive_timer_priv *priv = dev_get_priv(dev);int timer_channel;struct clk clk;int ret;priv->base = dev_read_addr_ptr(dev);if (IS_ERR(priv->base))
if (!priv->base) return -EINVAL
return PTR_ERR(priv->base);timer_channel = dev_read_u32_default(dev, "channel", 0);priv->base = priv->base + (0x40 * timer_channel);/* Get clock rate from channel selectecd*/ret = clk_get_by_index(dev, timer_channel, &clk);if (ret)return ret;ret = clk_enable(&clk);if (ret)return ret;uc_priv->clock_rate = clk_get_rate(&clk);/* Initiate timer, channel 0 *//* Unmask Interrupt Mask */
multi-line comment style is:
/* * line 1 * line 2 */
writel(0, priv->base + STF_TIMER_INT_MASK);/* Single run mode Setting */if (dev_read_bool(dev, "single-run"))writel(1, priv->base + STF_TIMER_CTL);/* Set Reload value */priv->timer_size = dev_read_u32_default(dev, "timer-size", 0xffffffff);
-1U ?
writel(priv->timer_size, priv->base + STF_TIMER_LOAD);/* Enable to start timer */writel(1, priv->base + STF_TIMER_ENABLE);return 0;+}
+static const struct udevice_id starfive_ids[] = {
{ .compatible = "starfive,jh8100-timers" },{ }+};
+U_BOOT_DRIVER(jh8100_starfive_timer) = {
.name = "jh8100_starfive_timer",
What is jh8100 ? Do you need that?
.id = UCLASS_TIMER,.of_match = starfive_ids,.probe = starfive_probe,.ops = &starfive_ops,.priv_auto = sizeof(struct starfive_timer_priv),+};
2.34.1
Regards, Simon
Hi Simon,
-----Original Message----- From: Simon Glass sjg@google.com Sent: Wednesday, October 4, 2023 10:11 AM To: KuanLim.Lee kuanlim.lee@starfivetech.com Cc: u-boot@lists.denx.de; WeiLiang Lim weiliang.lim@starfivetech.com Subject: Re: [PATCH] timer: starfive: Add Starfive timer support
On Tue, 19 Sept 2023 at 06:08, Kuan Lim Lee kuanlim.lee@starfivetech.com wrote:
Add timer driver in Starfive SoC. It is an timer that outside of CPU core and inside Starfive SoC.
Signed-off-by: Kuan Lim Lee kuanlim.lee@starfivetech.com Reviewed-by: Wei Liang Lim weiliang.lim@starfivetech.com
drivers/timer/Kconfig | 7 +++ drivers/timer/Makefile | 1 + drivers/timer/starfive-timer.c | 94 ++++++++++++++++++++++++++++++++++ 3 files changed, 102 insertions(+) create mode 100644 drivers/timer/starfive-timer.c
Reviewed-by: Simon Glass sjg@chromium.org
nits below
What are the nits? How do the nits to be solved?
diff --git a/drivers/timer/Kconfig b/drivers/timer/Kconfig index 915b2af160..a98be9dfae 100644 --- a/drivers/timer/Kconfig +++ b/drivers/timer/Kconfig @@ -326,4 +326,11 @@ config XILINX_TIMER Select this to enable support for the timer found on any Xilinx boards (axi timer).
+config STARFIVE_TIMER
bool "Starfive timer support"depends on TIMERhelpSelect this to enable support for the timer found onStarfive SoC.What resolution is the timer? How is it clocked? Is there only one channel?
Timer will be driven by clock pulses from clocks. The clocks are defined in device tree. Four channels timer, each timer has own clock source. Clock source frequency is gotten by clk_get_by_index () and clk_get_rate(). The timer counter register is a 32bits size register.
endmenu diff --git a/drivers/timer/Makefile b/drivers/timer/Makefile index 1ca74805fd..1ef814970b 100644 --- a/drivers/timer/Makefile +++ b/drivers/timer/Makefile @@ -34,3 +34,4 @@ obj-$(CONFIG_MTK_TIMER) += mtk_timer.o obj-$(CONFIG_MCHP_PIT64B_TIMER) += mchp-pit64b-timer.o obj-$(CONFIG_IMX_GPT_TIMER) += imx-gpt-timer.o obj-$(CONFIG_XILINX_TIMER) += xilinx-timer.o +obj-$(CONFIG_STARFIVE_TIMER) += starfive-timer.o diff --git a/drivers/timer/starfive-timer.c b/drivers/timer/starfive-timer.c new file mode 100644 index 0000000000..816402fdbf --- /dev/null +++ b/drivers/timer/starfive-timer.c @@ -0,0 +1,94 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright 2022 StarFive, Inc. All rights reserved.
- Author: Lee Kuan Lim kuanlim.lee@starfivetech.com
- */
+#include <common.h> +#include <clk.h> +#include <dm.h> +#include <time.h> +#include <timer.h> +#include <asm/io.h> +#include <dm/device-internal.h> +#include <linux/err.h>
+#define STF_TIMER_INT_STATUS 0x00 +#define STF_TIMER_CTL 0x04 +#define STF_TIMER_LOAD 0x08 +#define STF_TIMER_ENABLE 0x10 +#define STF_TIMER_RELOAD 0x14 +#define STF_TIMER_VALUE 0x18 +#define STF_TIMER_INT_CLR 0x20 +#define STF_TIMER_INT_MASK 0x24
+struct starfive_timer_priv {
void __iomem *base;u32 timer_size;+};
+static u64 notrace starfive_get_count(struct udevice *dev) {
struct starfive_timer_priv *priv = dev_get_priv(dev);/* Read decrement timer value and convert to increment value */return priv->timer_size - readl(priv->base + STF_TIMER_VALUE);+}
As an enhancement, you could provide a timer_early_get_count() function and an easly setup, so you can use bootstage.
Is this a must? If so, I must define some fixed address to get the timer count, enable timer, clock source frequency because I can't get base address and frequency from the device tree in early stage.
+static const struct timer_ops starfive_ops = {
.get_count = starfive_get_count, };+static int starfive_probe(struct udevice *dev) {
struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev);struct starfive_timer_priv *priv = dev_get_priv(dev);int timer_channel;struct clk clk;int ret;priv->base = dev_read_addr_ptr(dev);if (IS_ERR(priv->base))if (!priv->base) return -EINVAL
Noted.
return PTR_ERR(priv->base);timer_channel = dev_read_u32_default(dev, "channel", 0);priv->base = priv->base + (0x40 * timer_channel);/* Get clock rate from channel selectecd*/ret = clk_get_by_index(dev, timer_channel, &clk);if (ret)return ret;ret = clk_enable(&clk);if (ret)return ret;uc_priv->clock_rate = clk_get_rate(&clk);/* Initiate timer, channel 0 *//* Unmask Interrupt Mask */multi-line comment style is:
/*
- line 1
- line 2
*/
Noted.
writel(0, priv->base + STF_TIMER_INT_MASK);/* Single run mode Setting */if (dev_read_bool(dev, "single-run"))writel(1, priv->base + STF_TIMER_CTL);/* Set Reload value */priv->timer_size = dev_read_u32_default(dev, "timer-size",- 0xffffffff);
-1U ?
Will replace 0xffffffff to -1U
writel(priv->timer_size, priv->base + STF_TIMER_LOAD);/* Enable to start timer */writel(1, priv->base + STF_TIMER_ENABLE);return 0;+}
+static const struct udevice_id starfive_ids[] = {
{ .compatible = "starfive,jh8100-timers" },{ }+};
+U_BOOT_DRIVER(jh8100_starfive_timer) = {
.name = "jh8100_starfive_timer",
Will use name " starfive_timer" instead of "jh8100_starfive_timer"
What is jh8100 ? Do you need that?
.id = UCLASS_TIMER,.of_match = starfive_ids,.probe = starfive_probe,.ops = &starfive_ops,.priv_auto = sizeof(struct starfive_timer_priv),+};
2.34.1
Regards, Simon
Hi Simon,
Do my questions and replies are proper? Sorry for this time is my first time upstream.
-----Original Message----- From: KuanLim.Lee Sent: Wednesday, October 4, 2023 5:49 PM To: 'Simon Glass' sjg@google.com Cc: u-boot@lists.denx.de; WeiLiang Lim weiliang.lim@starfivetech.com Subject: RE: [PATCH] timer: starfive: Add Starfive timer support
Hi Simon,
-----Original Message----- From: Simon Glass sjg@google.com Sent: Wednesday, October 4, 2023 10:11 AM To: KuanLim.Lee kuanlim.lee@starfivetech.com Cc: u-boot@lists.denx.de; WeiLiang Lim weiliang.lim@starfivetech.com Subject: Re: [PATCH] timer: starfive: Add Starfive timer support
On Tue, 19 Sept 2023 at 06:08, Kuan Lim Lee kuanlim.lee@starfivetech.com wrote:
Add timer driver in Starfive SoC. It is an timer that outside of CPU core and inside Starfive SoC.
Signed-off-by: Kuan Lim Lee kuanlim.lee@starfivetech.com Reviewed-by: Wei Liang Lim weiliang.lim@starfivetech.com
drivers/timer/Kconfig | 7 +++ drivers/timer/Makefile | 1 + drivers/timer/starfive-timer.c | 94 ++++++++++++++++++++++++++++++++++ 3 files changed, 102 insertions(+) create mode 100644 drivers/timer/starfive-timer.c
Reviewed-by: Simon Glass sjg@chromium.org
nits below
What are the nits? How do the nits to be solved?
diff --git a/drivers/timer/Kconfig b/drivers/timer/Kconfig index 915b2af160..a98be9dfae 100644 --- a/drivers/timer/Kconfig +++ b/drivers/timer/Kconfig @@ -326,4 +326,11 @@ config XILINX_TIMER Select this to enable support for the timer found on any Xilinx boards (axi timer).
+config STARFIVE_TIMER
bool "Starfive timer support"depends on TIMERhelpSelect this to enable support for the timer found onStarfive SoC.What resolution is the timer? How is it clocked? Is there only one channel?
Timer will be driven by clock pulses from clocks. The clocks are defined in device tree. Four channels timer, each timer has own clock source. Clock source frequency is gotten by clk_get_by_index () and clk_get_rate(). The timer counter register is a 32bits size register.
endmenu diff --git a/drivers/timer/Makefile b/drivers/timer/Makefile index 1ca74805fd..1ef814970b 100644 --- a/drivers/timer/Makefile +++ b/drivers/timer/Makefile @@ -34,3 +34,4 @@ obj-$(CONFIG_MTK_TIMER) += mtk_timer.o obj-$(CONFIG_MCHP_PIT64B_TIMER) += mchp-pit64b-timer.o obj-$(CONFIG_IMX_GPT_TIMER) += imx-gpt-timer.o obj-$(CONFIG_XILINX_TIMER) += xilinx-timer.o +obj-$(CONFIG_STARFIVE_TIMER) += starfive-timer.o diff --git a/drivers/timer/starfive-timer.c b/drivers/timer/starfive-timer.c new file mode 100644 index 0000000000..816402fdbf --- /dev/null +++ b/drivers/timer/starfive-timer.c @@ -0,0 +1,94 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright 2022 StarFive, Inc. All rights reserved.
- Author: Lee Kuan Lim kuanlim.lee@starfivetech.com
- */
+#include <common.h> +#include <clk.h> +#include <dm.h> +#include <time.h> +#include <timer.h> +#include <asm/io.h> +#include <dm/device-internal.h> +#include <linux/err.h>
+#define STF_TIMER_INT_STATUS 0x00 +#define STF_TIMER_CTL 0x04 +#define STF_TIMER_LOAD 0x08 +#define STF_TIMER_ENABLE 0x10 +#define STF_TIMER_RELOAD 0x14 +#define STF_TIMER_VALUE 0x18 +#define STF_TIMER_INT_CLR 0x20 +#define STF_TIMER_INT_MASK 0x24
+struct starfive_timer_priv {
void __iomem *base;u32 timer_size;+};
+static u64 notrace starfive_get_count(struct udevice *dev) {
struct starfive_timer_priv *priv = dev_get_priv(dev);/* Read decrement timer value and convert to increment value */return priv->timer_size - readl(priv->base ++STF_TIMER_VALUE); }
As an enhancement, you could provide a timer_early_get_count() function and an easly setup, so you can use bootstage.
Is this a must? If so, I must define some fixed address to get the timer count, enable timer, clock source frequency because I can't get base address and frequency from the device tree in early stage.
+static const struct timer_ops starfive_ops = {
.get_count = starfive_get_count, };+static int starfive_probe(struct udevice *dev) {
struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev);struct starfive_timer_priv *priv = dev_get_priv(dev);int timer_channel;struct clk clk;int ret;priv->base = dev_read_addr_ptr(dev);if (IS_ERR(priv->base))if (!priv->base) return -EINVAL
Noted.
return PTR_ERR(priv->base);timer_channel = dev_read_u32_default(dev, "channel", 0);priv->base = priv->base + (0x40 * timer_channel);/* Get clock rate from channel selectecd*/ret = clk_get_by_index(dev, timer_channel, &clk);if (ret)return ret;ret = clk_enable(&clk);if (ret)return ret;uc_priv->clock_rate = clk_get_rate(&clk);/* Initiate timer, channel 0 *//* Unmask Interrupt Mask */multi-line comment style is:
/*
- line 1
- line 2
*/
Noted.
writel(0, priv->base + STF_TIMER_INT_MASK);/* Single run mode Setting */if (dev_read_bool(dev, "single-run"))writel(1, priv->base + STF_TIMER_CTL);/* Set Reload value */priv->timer_size = dev_read_u32_default(dev, "timer-size",- 0xffffffff);
-1U ?
Will replace 0xffffffff to -1U
writel(priv->timer_size, priv->base + STF_TIMER_LOAD);/* Enable to start timer */writel(1, priv->base + STF_TIMER_ENABLE);return 0;+}
+static const struct udevice_id starfive_ids[] = {
{ .compatible = "starfive,jh8100-timers" },{ }+};
+U_BOOT_DRIVER(jh8100_starfive_timer) = {
.name = "jh8100_starfive_timer",Will use name " starfive_timer" instead of "jh8100_starfive_timer"
What is jh8100 ? Do you need that?
.id = UCLASS_TIMER,.of_match = starfive_ids,.probe = starfive_probe,.ops = &starfive_ops,.priv_auto = sizeof(struct starfive_timer_priv),+};
2.34.1
Regards, Simon
Hi,
On Wed, 4 Oct 2023 at 03:49, KuanLim.Lee kuanlim.lee@starfivetech.com wrote:
Hi Simon,
-----Original Message----- From: Simon Glass sjg@google.com Sent: Wednesday, October 4, 2023 10:11 AM To: KuanLim.Lee kuanlim.lee@starfivetech.com Cc: u-boot@lists.denx.de; WeiLiang Lim weiliang.lim@starfivetech.com Subject: Re: [PATCH] timer: starfive: Add Starfive timer support
On Tue, 19 Sept 2023 at 06:08, Kuan Lim Lee kuanlim.lee@starfivetech.com wrote:
Add timer driver in Starfive SoC. It is an timer that outside of CPU core and inside Starfive SoC.
Signed-off-by: Kuan Lim Lee kuanlim.lee@starfivetech.com Reviewed-by: Wei Liang Lim weiliang.lim@starfivetech.com
drivers/timer/Kconfig | 7 +++ drivers/timer/Makefile | 1 + drivers/timer/starfive-timer.c | 94 ++++++++++++++++++++++++++++++++++ 3 files changed, 102 insertions(+) create mode 100644 drivers/timer/starfive-timer.c
Reviewed-by: Simon Glass sjg@chromium.org
nits below
What are the nits? How do the nits to be solved?
they are the things I mentioned below
diff --git a/drivers/timer/Kconfig b/drivers/timer/Kconfig index 915b2af160..a98be9dfae 100644 --- a/drivers/timer/Kconfig +++ b/drivers/timer/Kconfig @@ -326,4 +326,11 @@ config XILINX_TIMER Select this to enable support for the timer found on any Xilinx boards (axi timer).
+config STARFIVE_TIMER
bool "Starfive timer support"depends on TIMERhelpSelect this to enable support for the timer found onStarfive SoC.What resolution is the timer? How is it clocked? Is there only one channel?
Timer will be driven by clock pulses from clocks. The clocks are defined in device tree. Four channels timer, each timer has own clock source. Clock source frequency is gotten by clk_get_by_index () and clk_get_rate(). The timer counter register is a 32bits size register.
OK so could you put these details in the help?
endmenu diff --git a/drivers/timer/Makefile b/drivers/timer/Makefile index 1ca74805fd..1ef814970b 100644 --- a/drivers/timer/Makefile +++ b/drivers/timer/Makefile @@ -34,3 +34,4 @@ obj-$(CONFIG_MTK_TIMER) += mtk_timer.o obj-$(CONFIG_MCHP_PIT64B_TIMER) += mchp-pit64b-timer.o obj-$(CONFIG_IMX_GPT_TIMER) += imx-gpt-timer.o obj-$(CONFIG_XILINX_TIMER) += xilinx-timer.o +obj-$(CONFIG_STARFIVE_TIMER) += starfive-timer.o diff --git a/drivers/timer/starfive-timer.c b/drivers/timer/starfive-timer.c new file mode 100644 index 0000000000..816402fdbf --- /dev/null +++ b/drivers/timer/starfive-timer.c @@ -0,0 +1,94 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright 2022 StarFive, Inc. All rights reserved.
- Author: Lee Kuan Lim kuanlim.lee@starfivetech.com
- */
+#include <common.h> +#include <clk.h> +#include <dm.h> +#include <time.h> +#include <timer.h> +#include <asm/io.h> +#include <dm/device-internal.h> +#include <linux/err.h>
+#define STF_TIMER_INT_STATUS 0x00 +#define STF_TIMER_CTL 0x04 +#define STF_TIMER_LOAD 0x08 +#define STF_TIMER_ENABLE 0x10 +#define STF_TIMER_RELOAD 0x14 +#define STF_TIMER_VALUE 0x18 +#define STF_TIMER_INT_CLR 0x20 +#define STF_TIMER_INT_MASK 0x24
+struct starfive_timer_priv {
void __iomem *base;u32 timer_size;+};
+static u64 notrace starfive_get_count(struct udevice *dev) {
struct starfive_timer_priv *priv = dev_get_priv(dev);/* Read decrement timer value and convert to increment value */return priv->timer_size - readl(priv->base + STF_TIMER_VALUE);+}
As an enhancement, you could provide a timer_early_get_count() function and an easly setup, so you can use bootstage.
Is this a must? If so, I must define some fixed address to get the timer count, enable timer, clock source frequency because I can't get base address and frequency from the device tree in early stage.
No it isn't. It is useful for bootstage though. I suspect you can read DT early, though.
+static const struct timer_ops starfive_ops = {
.get_count = starfive_get_count, };+static int starfive_probe(struct udevice *dev) {
struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev);struct starfive_timer_priv *priv = dev_get_priv(dev);int timer_channel;struct clk clk;int ret;priv->base = dev_read_addr_ptr(dev);if (IS_ERR(priv->base))if (!priv->base) return -EINVAL
Noted.
return PTR_ERR(priv->base);timer_channel = dev_read_u32_default(dev, "channel", 0);priv->base = priv->base + (0x40 * timer_channel);/* Get clock rate from channel selectecd*/ret = clk_get_by_index(dev, timer_channel, &clk);if (ret)return ret;ret = clk_enable(&clk);if (ret)return ret;uc_priv->clock_rate = clk_get_rate(&clk);/* Initiate timer, channel 0 *//* Unmask Interrupt Mask */multi-line comment style is:
/*
- line 1
- line 2
*/
Noted.
writel(0, priv->base + STF_TIMER_INT_MASK);/* Single run mode Setting */if (dev_read_bool(dev, "single-run"))writel(1, priv->base + STF_TIMER_CTL);/* Set Reload value */priv->timer_size = dev_read_u32_default(dev, "timer-size",- 0xffffffff);
-1U ?
Will replace 0xffffffff to -1U
writel(priv->timer_size, priv->base + STF_TIMER_LOAD);/* Enable to start timer */writel(1, priv->base + STF_TIMER_ENABLE);return 0;+}
+static const struct udevice_id starfive_ids[] = {
{ .compatible = "starfive,jh8100-timers" },{ }+};
+U_BOOT_DRIVER(jh8100_starfive_timer) = {
.name = "jh8100_starfive_timer",Will use name " starfive_timer" instead of "jh8100_starfive_timer"
What is jh8100 ? Do you need that?
.id = UCLASS_TIMER,.of_match = starfive_ids,.probe = starfive_probe,.ops = &starfive_ops,.priv_auto = sizeof(struct starfive_timer_priv),+};
2.34.1
Regards, Simon
Hi Simon,
If there are no more issue, I will send out version 2 patch.
-----Original Message----- From: Simon Glass sjg@google.com Sent: Tuesday, October 10, 2023 9:45 PM To: KuanLim.Lee kuanlim.lee@starfivetech.com Cc: u-boot@lists.denx.de; WeiLiang Lim weiliang.lim@starfivetech.com Subject: Re: [PATCH] timer: starfive: Add Starfive timer support
Hi,
On Wed, 4 Oct 2023 at 03:49, KuanLim.Lee kuanlim.lee@starfivetech.com wrote:
Hi Simon,
-----Original Message----- From: Simon Glass sjg@google.com Sent: Wednesday, October 4, 2023 10:11 AM To: KuanLim.Lee kuanlim.lee@starfivetech.com Cc: u-boot@lists.denx.de; WeiLiang Lim weiliang.lim@starfivetech.com Subject: Re: [PATCH] timer: starfive: Add Starfive timer support
On Tue, 19 Sept 2023 at 06:08, Kuan Lim Lee kuanlim.lee@starfivetech.com wrote:
Add timer driver in Starfive SoC. It is an timer that outside of CPU core and inside Starfive SoC.
Signed-off-by: Kuan Lim Lee kuanlim.lee@starfivetech.com Reviewed-by: Wei Liang Lim weiliang.lim@starfivetech.com
drivers/timer/Kconfig | 7 +++ drivers/timer/Makefile | 1 + drivers/timer/starfive-timer.c | 94 ++++++++++++++++++++++++++++++++++ 3 files changed, 102 insertions(+) create mode 100644 drivers/timer/starfive-timer.c
Reviewed-by: Simon Glass sjg@chromium.org
nits below
What are the nits? How do the nits to be solved?
they are the things I mentioned below
diff --git a/drivers/timer/Kconfig b/drivers/timer/Kconfig index 915b2af160..a98be9dfae 100644 --- a/drivers/timer/Kconfig +++ b/drivers/timer/Kconfig @@ -326,4 +326,11 @@ config XILINX_TIMER Select this to enable support for the timer found on any Xilinx boards (axi timer).
+config STARFIVE_TIMER
bool "Starfive timer support"depends on TIMERhelpSelect this to enable support for the timer found onStarfive SoC.What resolution is the timer? How is it clocked? Is there only one channel?
Timer will be driven by clock pulses from clocks. The clocks are defined in
device tree.
Four channels timer, each timer has own clock source. Clock source frequency is gotten by clk_get_by_index () and clk_get_rate(). The timer counter register is a 32bits size register.
OK so could you put these details in the help?
Timer0 bias = 0x00, Timer1 bias = 0x40, Timer2 bias = 0x80, Timer3 bias = 0xc0
Register: TIMER Interrupts Status (offset = 0x00) Bit0 - timer_int_ch0 - default: 0x00 Bit1 - timer_int_ch1 - default: 0x00 Bit2 - timer_int_ch2 - default: 0x00 Bit3 - timer_int_ch3 - default: 0x00
TIMER Control Register (offset = 0x04+bias) Bit[0] - default: 0x0 0: continuous run, 1: single run Single run mode will stop after the TIMER Value Register is counted to zero and reloaded. Continuous run mode will keep on running after the TIMER Value Register is counted to zero and reloaded.
TIMER Load Register (offset = 0x08+bias) Bit[31:0] - default: 0xffffffff The value is always reloaded into TIMER Value Register
TIMER Enable Register (offset = 0x10+bias) Bit[0] - default: 0x0 0: disable, 1: enable
TIMER Reload Register (offset = 0x14+bias) Bit[0] - default: 0x0 Write this register to reload preset value to counter. (Write 0 and 1 are both ok) Read this register always get '0'.
TIMER Value Register (offset = 0x18+bias) Bit[31:0] - default: 0xffffffff Indicates value of the 32-bit counter. Read-Only Register.
TIMER Interrupt Status/Clear Register (offset = 0x20+bias) Bit[1] - default: 0x0. "timer_int_clr_busy". Read-Only bit. '1' means that it is clearing interrupt. Bit 0 can NOT be written. '0' means Bit 0 can be written to clear interrupt. Bit[0] - default: 0x0. "timer_int_clear_status". Read-Write bit. Read value represent channel interrupt status. Write 1 to this bit clear interrupt. Write 0 has no effects.
TIMER Interrupt Mask Register (offset = 0x24+bias) Bit[0] - default: 0x1 Interrupt Mask Register. 0: Unmask, 1: Mask It must be unmasked to before using the timer channel.
Current device tree will put the clock source details. So that, driver can get the clock source frequency of the channel by clk_get_by_index () and clk_get_rate(). clock-names = "ch0", "ch1", "ch2", "ch3", "apb";
I don’t use interrupt here and only choose one of the timer channels to use. Please tell me if these details are not sufficient.
endmenu diff --git a/drivers/timer/Makefile b/drivers/timer/Makefile index 1ca74805fd..1ef814970b 100644 --- a/drivers/timer/Makefile +++ b/drivers/timer/Makefile @@ -34,3 +34,4 @@ obj-$(CONFIG_MTK_TIMER) += mtk_timer.o obj-$(CONFIG_MCHP_PIT64B_TIMER) += mchp-pit64b-timer.o obj-$(CONFIG_IMX_GPT_TIMER) += imx-gpt-timer.o obj-$(CONFIG_XILINX_TIMER) += xilinx-timer.o +obj-$(CONFIG_STARFIVE_TIMER) += starfive-timer.o diff --git a/drivers/timer/starfive-timer.c b/drivers/timer/starfive-timer.c new file mode 100644 index 0000000000..816402fdbf --- /dev/null +++ b/drivers/timer/starfive-timer.c @@ -0,0 +1,94 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright 2022 StarFive, Inc. All rights reserved.
- Author: Lee Kuan Lim kuanlim.lee@starfivetech.com
- */
+#include <common.h> +#include <clk.h> +#include <dm.h> +#include <time.h> +#include <timer.h> +#include <asm/io.h> +#include <dm/device-internal.h> +#include <linux/err.h>
+#define STF_TIMER_INT_STATUS 0x00 +#define STF_TIMER_CTL 0x04 +#define STF_TIMER_LOAD 0x08 +#define STF_TIMER_ENABLE 0x10 +#define STF_TIMER_RELOAD 0x14 +#define STF_TIMER_VALUE 0x18 +#define STF_TIMER_INT_CLR 0x20 +#define STF_TIMER_INT_MASK 0x24
+struct starfive_timer_priv {
void __iomem *base;u32 timer_size;+};
+static u64 notrace starfive_get_count(struct udevice *dev) {
struct starfive_timer_priv *priv = dev_get_priv(dev);/* Read decrement timer value and convert to increment value */return priv->timer_size - readl(priv->base ++STF_TIMER_VALUE); }
As an enhancement, you could provide a timer_early_get_count() function and an easly setup, so you can use bootstage.
Is this a must? If so, I must define some fixed address to get the timer count, enable timer, clock source frequency because I can't get base address and frequency from the device tree in early stage.
No it isn't. It is useful for bootstage though. I suspect you can read DT early, though.
Current driver is working well and not going to change in current stage. I will enhance the driver if need to trace time before driver model.
+static const struct timer_ops starfive_ops = {
.get_count = starfive_get_count, };+static int starfive_probe(struct udevice *dev) {
struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev);struct starfive_timer_priv *priv = dev_get_priv(dev);int timer_channel;struct clk clk;int ret;priv->base = dev_read_addr_ptr(dev);if (IS_ERR(priv->base))if (!priv->base) return -EINVAL
Noted.
return PTR_ERR(priv->base);timer_channel = dev_read_u32_default(dev, "channel", 0);priv->base = priv->base + (0x40 * timer_channel);/* Get clock rate from channel selectecd*/ret = clk_get_by_index(dev, timer_channel, &clk);if (ret)return ret;ret = clk_enable(&clk);if (ret)return ret;uc_priv->clock_rate = clk_get_rate(&clk);/* Initiate timer, channel 0 *//* Unmask Interrupt Mask */multi-line comment style is:
/*
- line 1
- line 2
*/
Noted.
writel(0, priv->base + STF_TIMER_INT_MASK);/* Single run mode Setting */if (dev_read_bool(dev, "single-run"))writel(1, priv->base + STF_TIMER_CTL);/* Set Reload value */priv->timer_size = dev_read_u32_default(dev, "timer-size",- 0xffffffff);
-1U ?
Will replace 0xffffffff to -1U
writel(priv->timer_size, priv->base + STF_TIMER_LOAD);/* Enable to start timer */writel(1, priv->base + STF_TIMER_ENABLE);return 0;+}
+static const struct udevice_id starfive_ids[] = {
{ .compatible = "starfive,jh8100-timers" },{ }+};
+U_BOOT_DRIVER(jh8100_starfive_timer) = {
.name = "jh8100_starfive_timer",Will use name " starfive_timer" instead of "jh8100_starfive_timer"
What is jh8100 ? Do you need that?
.id = UCLASS_TIMER,.of_match = starfive_ids,.probe = starfive_probe,.ops = &starfive_ops,.priv_auto = sizeof(struct starfive_timer_priv),+};
2.34.1
Regards, Simon
On Tues, 10 Oct 2023 at 21:45, Simon Glass sjg@google.com wrote:
On Wed, 4 Oct 2023 at 03:49, KuanLim.Lee kuanlim.lee@starfivetech.com wrote:
Hi Simon, On Wed, 4 Oct 2023 at 10:11, Simon Glass sjg@google.com wrote:
On Tue, 19 Sept 2023 at 06:08, Kuan Lim Lee kuanlim.lee@starfivetech.com wrote:
Add timer driver in Starfive SoC. It is an timer that outside of CPU core and inside Starfive SoC.
Signed-off-by: Kuan Lim Lee kuanlim.lee@starfivetech.com Reviewed-by: Wei Liang Lim weiliang.lim@starfivetech.com
drivers/timer/Kconfig | 7 +++ drivers/timer/Makefile | 1 + drivers/timer/starfive-timer.c | 94 ++++++++++++++++++++++++++++++++++ 3 files changed, 102 insertions(+) create mode 100644 drivers/timer/starfive-timer.c
Reviewed-by: Simon Glass sjg@chromium.org
nits below
What are the nits? How do the nits to be solved?
they are the things I mentioned below
diff --git a/drivers/timer/Kconfig b/drivers/timer/Kconfig index 915b2af160..a98be9dfae 100644 --- a/drivers/timer/Kconfig +++ b/drivers/timer/Kconfig @@ -326,4 +326,11 @@ config XILINX_TIMER Select this to enable support for the timer found on any Xilinx boards (axi timer).
+config STARFIVE_TIMER
bool "Starfive timer support"depends on TIMERhelpSelect this to enable support for the timer found onStarfive SoC.What resolution is the timer? How is it clocked? Is there only one channel?
Timer will be driven by clock pulses from clocks. The clocks are defined in
device tree.
Four channels timer, each timer has own clock source. Clock source frequency is gotten by clk_get_by_index () and clk_get_rate(). The timer counter register is a 32bits size register.
OK so could you put these details in the help?
Timer0 bias = 0x00, Timer1 bias = 0x40, Timer2 bias = 0x80, Timer3 bias = 0xc0
Register: TIMER Interrupts Status (offset = 0x00) Bit0 - timer_int_ch0 - default: 0x00 Bit1 - timer_int_ch1 - default: 0x00 Bit2 - timer_int_ch2 - default: 0x00 Bit3 - timer_int_ch3 - default: 0x00
TIMER Control Register (offset = 0x04+bias) Bit[0] - default: 0x0 0: continuous run, 1: single run Single run mode will stop after the TIMER Value Register is counted to zero and reloaded. Continuous run mode will keep on running after the TIMER Value Register is counted to zero and reloaded.
TIMER Load Register (offset = 0x08+bias) Bit[31:0] - default: 0xffffffff The value is always reloaded into TIMER Value Register
TIMER Enable Register (offset = 0x10+bias) Bit[0] - default: 0x0 0: disable, 1: enable
TIMER Reload Register (offset = 0x14+bias) Bit[0] - default: 0x0 Write this register to reload preset value to counter. (Write 0 and 1 are both ok) Read this register always get '0'.
TIMER Value Register (offset = 0x18+bias) Bit[31:0] - default: 0xffffffff Indicates value of the 32-bit counter. Read-Only Register.
TIMER Interrupt Status/Clear Register (offset = 0x20+bias) Bit[1] - default: 0x0. "timer_int_clr_busy". Read-Only bit. '1' means that it is clearing interrupt. Bit 0 can NOT be written. '0' means Bit 0 can be written to clear interrupt. Bit[0] - default: 0x0. "timer_int_clear_status". Read-Write bit. Read value represent channel interrupt status. Write 1 to this bit clear interrupt. Write 0 has no effects.
TIMER Interrupt Mask Register (offset = 0x24+bias) Bit[0] - default: 0x1 Interrupt Mask Register. 0: Unmask, 1: Mask It must be unmasked to before using the timer channel.
Current device tree will put the clock source details. So that, driver can get the clock source frequency of the channel by clk_get_by_index () and clk_get_rate(). clock-names = "ch0", "ch1", "ch2", "ch3", "apb";
I don’t use interrupt here and only choose one of the timer channels to use. Please tell me if these details are not sufficient.
endmenu diff --git a/drivers/timer/Makefile b/drivers/timer/Makefile index 1ca74805fd..1ef814970b 100644 --- a/drivers/timer/Makefile +++ b/drivers/timer/Makefile @@ -34,3 +34,4 @@ obj-$(CONFIG_MTK_TIMER) += mtk_timer.o obj-$(CONFIG_MCHP_PIT64B_TIMER) += mchp-pit64b-timer.o obj-$(CONFIG_IMX_GPT_TIMER) += imx-gpt-timer.o obj-$(CONFIG_XILINX_TIMER) += xilinx-timer.o +obj-$(CONFIG_STARFIVE_TIMER) += starfive-timer.o diff --git a/drivers/timer/starfive-timer.c b/drivers/timer/starfive-timer.c new file mode 100644 index 0000000000..816402fdbf --- /dev/null +++ b/drivers/timer/starfive-timer.c @@ -0,0 +1,94 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright 2022 StarFive, Inc. All rights reserved.
- Author: Lee Kuan Lim kuanlim.lee@starfivetech.com
- */
+#include <common.h> +#include <clk.h> +#include <dm.h> +#include <time.h> +#include <timer.h> +#include <asm/io.h> +#include <dm/device-internal.h> #include <linux/err.h>
+#define STF_TIMER_INT_STATUS 0x00 +#define STF_TIMER_CTL 0x04 +#define STF_TIMER_LOAD 0x08 +#define STF_TIMER_ENABLE 0x10 +#define STF_TIMER_RELOAD 0x14 +#define STF_TIMER_VALUE 0x18 +#define STF_TIMER_INT_CLR 0x20 +#define STF_TIMER_INT_MASK 0x24
+struct starfive_timer_priv {
void __iomem *base;u32 timer_size;+};
+static u64 notrace starfive_get_count(struct udevice *dev) {
struct starfive_timer_priv *priv = dev_get_priv(dev);/* Read decrement timer value and convert to increment value */return priv->timer_size - readl(priv->base ++STF_TIMER_VALUE); }
As an enhancement, you could provide a timer_early_get_count() function and an easly setup, so you can use bootstage.
Is this a must? If so, I must define some fixed address to get the timer count, enable timer, clock source frequency because I can't get base address and frequency from the device tree in early stage.
No it isn't. It is useful for bootstage though. I suspect you can read DT early, though.
Current driver is working well and not going to change in current stage. I will enhance the driver if need to trace time before driver model.
+static const struct timer_ops starfive_ops = {
.get_count = starfive_get_count, };+static int starfive_probe(struct udevice *dev) {
struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev);struct starfive_timer_priv *priv = dev_get_priv(dev);int timer_channel;struct clk clk;int ret;priv->base = dev_read_addr_ptr(dev);if (IS_ERR(priv->base))if (!priv->base) return -EINVAL
Noted.
return PTR_ERR(priv->base);timer_channel = dev_read_u32_default(dev, "channel", 0);priv->base = priv->base + (0x40 * timer_channel);/* Get clock rate from channel selectecd*/ret = clk_get_by_index(dev, timer_channel, &clk);if (ret)return ret;ret = clk_enable(&clk);if (ret)return ret;uc_priv->clock_rate = clk_get_rate(&clk);/* Initiate timer, channel 0 *//* Unmask Interrupt Mask */multi-line comment style is:
/*
- line 1
- line 2
*/
Noted.
writel(0, priv->base + STF_TIMER_INT_MASK);/* Single run mode Setting */if (dev_read_bool(dev, "single-run"))writel(1, priv->base + STF_TIMER_CTL);/* Set Reload value */priv->timer_size = dev_read_u32_default(dev,- "timer-size", 0xffffffff);
-1U ?
Will replace 0xffffffff to -1U
writel(priv->timer_size, priv->base + STF_TIMER_LOAD);/* Enable to start timer */writel(1, priv->base + STF_TIMER_ENABLE);return 0;+}
+static const struct udevice_id starfive_ids[] = {
{ .compatible = "starfive,jh8100-timers" },{ }+};
+U_BOOT_DRIVER(jh8100_starfive_timer) = {
.name = "jh8100_starfive_timer",Will use name " starfive_timer" instead of "jh8100_starfive_timer"
What is jh8100 ? Do you need that?
.id = UCLASS_TIMER,.of_match = starfive_ids,.probe = starfive_probe,.ops = &starfive_ops,.priv_auto = sizeof(struct starfive_timer_priv),+};
2.34.1
Regards, Simon
Hi Simon, Please tell me if I am doing something wrong, I will do my best to correct it. Best Regards, KuanLim Lee
Hi Simon,
Please be reminded that to give some review if it still has some issues.
-----Original Message----- From: KuanLim.Lee Sent: Wednesday, October 4, 2023 5:49 PM To: 'Simon Glass' sjg@google.com Cc: u-boot@lists.denx.de; WeiLiang Lim weiliang.lim@starfivetech.com Subject: RE: [PATCH] timer: starfive: Add Starfive timer support
Hi Simon,
-----Original Message----- From: Simon Glass sjg@google.com Sent: Wednesday, October 4, 2023 10:11 AM To: KuanLim.Lee kuanlim.lee@starfivetech.com Cc: u-boot@lists.denx.de; WeiLiang Lim weiliang.lim@starfivetech.com Subject: Re: [PATCH] timer: starfive: Add Starfive timer support
On Tue, 19 Sept 2023 at 06:08, Kuan Lim Lee kuanlim.lee@starfivetech.com wrote:
Add timer driver in Starfive SoC. It is an timer that outside of CPU core and inside Starfive SoC.
Signed-off-by: Kuan Lim Lee kuanlim.lee@starfivetech.com Reviewed-by: Wei Liang Lim weiliang.lim@starfivetech.com
drivers/timer/Kconfig | 7 +++ drivers/timer/Makefile | 1 + drivers/timer/starfive-timer.c | 94 ++++++++++++++++++++++++++++++++++ 3 files changed, 102 insertions(+) create mode 100644 drivers/timer/starfive-timer.c
Reviewed-by: Simon Glass sjg@chromium.org
nits below
What are the nits? How do the nits to be solved?
diff --git a/drivers/timer/Kconfig b/drivers/timer/Kconfig index 915b2af160..a98be9dfae 100644 --- a/drivers/timer/Kconfig +++ b/drivers/timer/Kconfig @@ -326,4 +326,11 @@ config XILINX_TIMER Select this to enable support for the timer found on any Xilinx boards (axi timer).
+config STARFIVE_TIMER
bool "Starfive timer support"depends on TIMERhelpSelect this to enable support for the timer found onStarfive SoC.What resolution is the timer? How is it clocked? Is there only one channel?
Timer will be driven by clock pulses from clocks. The clocks are defined in device tree. Four channels timer, each timer has own clock source. Clock source frequency is gotten by clk_get_by_index () and clk_get_rate(). The timer counter register is a 32bits size register.
endmenu diff --git a/drivers/timer/Makefile b/drivers/timer/Makefile index 1ca74805fd..1ef814970b 100644 --- a/drivers/timer/Makefile +++ b/drivers/timer/Makefile @@ -34,3 +34,4 @@ obj-$(CONFIG_MTK_TIMER) += mtk_timer.o obj-$(CONFIG_MCHP_PIT64B_TIMER) += mchp-pit64b-timer.o obj-$(CONFIG_IMX_GPT_TIMER) += imx-gpt-timer.o obj-$(CONFIG_XILINX_TIMER) += xilinx-timer.o +obj-$(CONFIG_STARFIVE_TIMER) += starfive-timer.o diff --git a/drivers/timer/starfive-timer.c b/drivers/timer/starfive-timer.c new file mode 100644 index 0000000000..816402fdbf --- /dev/null +++ b/drivers/timer/starfive-timer.c @@ -0,0 +1,94 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright 2022 StarFive, Inc. All rights reserved.
- Author: Lee Kuan Lim kuanlim.lee@starfivetech.com
- */
+#include <common.h> +#include <clk.h> +#include <dm.h> +#include <time.h> +#include <timer.h> +#include <asm/io.h> +#include <dm/device-internal.h> +#include <linux/err.h>
+#define STF_TIMER_INT_STATUS 0x00 +#define STF_TIMER_CTL 0x04 +#define STF_TIMER_LOAD 0x08 +#define STF_TIMER_ENABLE 0x10 +#define STF_TIMER_RELOAD 0x14 +#define STF_TIMER_VALUE 0x18 +#define STF_TIMER_INT_CLR 0x20 +#define STF_TIMER_INT_MASK 0x24
+struct starfive_timer_priv {
void __iomem *base;u32 timer_size;+};
+static u64 notrace starfive_get_count(struct udevice *dev) {
struct starfive_timer_priv *priv = dev_get_priv(dev);/* Read decrement timer value and convert to increment value */return priv->timer_size - readl(priv->base ++STF_TIMER_VALUE); }
As an enhancement, you could provide a timer_early_get_count() function and an easly setup, so you can use bootstage.
Is this a must? If so, I must define some fixed address to get the timer count, enable timer, clock source frequency because I can't get base address and frequency from the device tree in early stage.
+static const struct timer_ops starfive_ops = {
.get_count = starfive_get_count, };+static int starfive_probe(struct udevice *dev) {
struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev);struct starfive_timer_priv *priv = dev_get_priv(dev);int timer_channel;struct clk clk;int ret;priv->base = dev_read_addr_ptr(dev);if (IS_ERR(priv->base))if (!priv->base) return -EINVAL
Noted.
return PTR_ERR(priv->base);timer_channel = dev_read_u32_default(dev, "channel", 0);priv->base = priv->base + (0x40 * timer_channel);/* Get clock rate from channel selectecd*/ret = clk_get_by_index(dev, timer_channel, &clk);if (ret)return ret;ret = clk_enable(&clk);if (ret)return ret;uc_priv->clock_rate = clk_get_rate(&clk);/* Initiate timer, channel 0 *//* Unmask Interrupt Mask */multi-line comment style is:
/*
- line 1
- line 2
*/
Noted.
writel(0, priv->base + STF_TIMER_INT_MASK);/* Single run mode Setting */if (dev_read_bool(dev, "single-run"))writel(1, priv->base + STF_TIMER_CTL);/* Set Reload value */priv->timer_size = dev_read_u32_default(dev, "timer-size",- 0xffffffff);
-1U ?
Will replace 0xffffffff to -1U
writel(priv->timer_size, priv->base + STF_TIMER_LOAD);/* Enable to start timer */writel(1, priv->base + STF_TIMER_ENABLE);return 0;+}
+static const struct udevice_id starfive_ids[] = {
{ .compatible = "starfive,jh8100-timers" },{ }+};
+U_BOOT_DRIVER(jh8100_starfive_timer) = {
.name = "jh8100_starfive_timer",Will use name " starfive_timer" instead of "jh8100_starfive_timer"
What is jh8100 ? Do you need that?
.id = UCLASS_TIMER,.of_match = starfive_ids,.probe = starfive_probe,.ops = &starfive_ops,.priv_auto = sizeof(struct starfive_timer_priv),+};
2.34.1
Regards, Simon
participants (4)
-
Kuan Lim Lee -
KuanLim.Lee
-
Simon Glass -
WeiLiang Lim