All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] timer: Return a u64 directly
@ 2020-10-07 18:37 Sean Anderson
  2020-10-07 18:37 ` [PATCH v2 1/2] doc: Document timer API Sean Anderson
  2020-10-07 18:37 ` [PATCH v2 2/2] timer: Return count from timer_ops.get_count Sean Anderson
  0 siblings, 2 replies; 8+ messages in thread
From: Sean Anderson @ 2020-10-07 18:37 UTC (permalink / raw)
  To: u-boot

This series will conflict with [1]. One could be rebased on the other, depending
on what order they are merged.

[1] https://patchwork.ozlabs.org/project/uboot/list/?series=205908

Changes in v2:
- Document timer API with kerneldoc
- mchp-pit64b was added since v1, so convert it
- Document when get_count may be called, and what assumptions the timer
  subsystem makes about drivers

Sean Anderson (2):
  doc: Document timer API
  timer: Return count from timer_ops.get_count

 arch/riscv/lib/andes_plmt.c       |  6 ++--
 arch/riscv/lib/sifive_clint.c     |  6 ++--
 doc/api/index.rst                 |  1 +
 doc/api/timer.rst                 |  8 +++++
 drivers/timer/ag101p_timer.c      |  5 ++-
 drivers/timer/altera_timer.c      |  6 ++--
 drivers/timer/arc_timer.c         |  6 ++--
 drivers/timer/ast_timer.c         |  6 ++--
 drivers/timer/atcpit100_timer.c   |  5 ++-
 drivers/timer/atmel_pit_timer.c   |  6 ++--
 drivers/timer/cadence-ttc.c       |  6 ++--
 drivers/timer/dw-apb-timer.c      |  6 ++--
 drivers/timer/mchp-pit64b-timer.c |  6 ++--
 drivers/timer/mpc83xx_timer.c     |  6 ++--
 drivers/timer/mtk_timer.c         |  6 ++--
 drivers/timer/nomadik-mtu-timer.c |  6 ++--
 drivers/timer/omap-timer.c        |  6 ++--
 drivers/timer/ostm_timer.c        |  6 ++--
 drivers/timer/riscv_timer.c       | 21 ++++++------
 drivers/timer/rockchip_timer.c    |  5 ++-
 drivers/timer/sandbox_timer.c     |  6 ++--
 drivers/timer/sti-timer.c         |  6 ++--
 drivers/timer/stm32_timer.c       |  6 ++--
 drivers/timer/timer-uclass.c      |  3 +-
 drivers/timer/tsc_timer.c         |  6 ++--
 include/timer.h                   | 53 +++++++++++++++++--------------
 26 files changed, 91 insertions(+), 118 deletions(-)
 create mode 100644 doc/api/timer.rst

-- 
2.28.0

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 1/2] doc: Document timer API
  2020-10-07 18:37 [PATCH v2 0/2] timer: Return a u64 directly Sean Anderson
@ 2020-10-07 18:37 ` Sean Anderson
  2020-10-12  3:34   ` Simon Glass
  2020-10-23  0:27   ` Tom Rini
  2020-10-07 18:37 ` [PATCH v2 2/2] timer: Return count from timer_ops.get_count Sean Anderson
  1 sibling, 2 replies; 8+ messages in thread
From: Sean Anderson @ 2020-10-07 18:37 UTC (permalink / raw)
  To: u-boot

This adds kerneldocs for <timer.h>.

I don't know who should maintain doc/api/timer.rst, since the timer
subsystem seems to be maintained by SoC maintainers. MAINTAINERS is left
un-updated for the moment.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---

Changes in v2:
- New

 doc/api/index.rst |  1 +
 doc/api/timer.rst |  8 ++++++++
 include/timer.h   | 46 ++++++++++++++++++++++++----------------------
 3 files changed, 33 insertions(+), 22 deletions(-)
 create mode 100644 doc/api/timer.rst

diff --git a/doc/api/index.rst b/doc/api/index.rst
index b7eb5725f2..b8f37f0d86 100644
--- a/doc/api/index.rst
+++ b/doc/api/index.rst
@@ -11,4 +11,5 @@ U-Boot API documentation
    linker_lists
    rng
    serial
+   timer
    unicode
diff --git a/doc/api/timer.rst b/doc/api/timer.rst
new file mode 100644
index 0000000000..b0695174d7
--- /dev/null
+++ b/doc/api/timer.rst
@@ -0,0 +1,8 @@
+.. SPDX-License-Identifier: GPL-2.0+
+.. Copyright (C) 2020 Sean Anderson <seanga2@gmail.com>
+
+Timer Subsystem
+===============
+
+.. kernel-doc:: include/timer.h
+   :internal:
diff --git a/include/timer.h b/include/timer.h
index 8b9fa51c53..aa9d870619 100644
--- a/include/timer.h
+++ b/include/timer.h
@@ -6,12 +6,12 @@
 #ifndef _TIMER_H_
 #define _TIMER_H_
 
-/*
- * dm_timer_init - initialize a timer for time keeping. On success
+/**
+ * dm_timer_init() - initialize a timer for time keeping. On success
  * initializes gd->timer so that lib/timer can use it for future
  * referrence.
  *
- * @return - 0 on success or error number
+ * Return: 0 on success or error number
  */
 int dm_timer_init(void);
 
@@ -30,49 +30,51 @@ int dm_timer_init(void);
  */
 int timer_timebase_fallback(struct udevice *dev);
 
-/*
- * timer_conv_64 - convert 32-bit counter value to 64-bit
- *
+/**
+ * timer_conv_64() - convert 32-bit counter value to 64-bit
  * @count: 32-bit counter value
- * @return: 64-bit counter value
+ *
+ * Return: 64-bit counter value
  */
 u64 timer_conv_64(u32 count);
 
-/*
- * Get the current timer count
- *
+/**
+ * timer_get_count() - Get the current timer count
  * @dev: The timer device
  * @count: pointer that returns the current timer count
- * @return: 0 if OK, -ve on error
+ *
+ * Return: 0 if OK, -ve on error
  */
 int timer_get_count(struct udevice *dev, u64 *count);
 
-/*
- * Get the timer input clock frequency
- *
+/**
+ * timer_get_rate() - Get the timer input clock frequency
  * @dev: The timer device
- * @return: the timer input clock frequency
+ *
+ * Return: the timer input clock frequency
  */
 unsigned long timer_get_rate(struct udevice *dev);
 
-/*
+/**
  * struct timer_ops - Driver model timer operations
  *
  * The uclass interface is implemented by all timer devices which use
  * driver model.
  */
 struct timer_ops {
-	/*
-	 * Get the current timer count
+	/**
+	 * @get_count: Get the current timer count
 	 *
 	 * @dev: The timer device
+	 *
 	 * @count: pointer that returns the current 64-bit timer count
-	 * @return: 0 if OK, -ve on error
+	 *
+	 * Return: 0 if OK, -ve on error
 	 */
 	int (*get_count)(struct udevice *dev, u64 *count);
 };
 
-/*
+/**
  * struct timer_dev_priv - information about a device used by the uclass
  *
  * @clock_rate: the timer input clock frequency
@@ -84,7 +86,7 @@ struct timer_dev_priv {
 /**
  * timer_early_get_count() - Implement timer_get_count() before driver model
  *
- * If CONFIG_TIMER_EARLY is enabled, this function wil be called to return
+ * If ``CONFIG_TIMER_EARLY`` is enabled, this function wil be called to return
  * the current timer value before the proper driver model timer is ready.
  * It should be implemented by one of the timer values. This is mostly useful
  * for tracing.
@@ -94,7 +96,7 @@ u64 timer_early_get_count(void);
 /**
  * timer_early_get_rate() - Get the timer rate before driver model
  *
- * If CONFIG_TIMER_EARLY is enabled, this function wil be called to return
+ * If ``CONFIG_TIMER_EARLY`` is enabled, this function wil be called to return
  * the current timer rate in Hz before the proper driver model timer is ready.
  * It should be implemented by one of the timer values. This is mostly useful
  * for tracing. This corresponds to the clock_rate value in struct
-- 
2.28.0

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 2/2] timer: Return count from timer_ops.get_count
  2020-10-07 18:37 [PATCH v2 0/2] timer: Return a u64 directly Sean Anderson
  2020-10-07 18:37 ` [PATCH v2 1/2] doc: Document timer API Sean Anderson
@ 2020-10-07 18:37 ` Sean Anderson
  2020-10-08  8:28   ` Claudiu.Beznea at microchip.com
                     ` (2 more replies)
  1 sibling, 3 replies; 8+ messages in thread
From: Sean Anderson @ 2020-10-07 18:37 UTC (permalink / raw)
  To: u-boot

No timer drivers return an error from get_count. Instead of possibly
returning an error, just return the count directly.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---

Changes in v2:
- mchp-pit64b was added since v1, so convert it
- Document when get_count may be called, and what assumptions the timer
  subsystem makes about drivers

 arch/riscv/lib/andes_plmt.c       |  6 ++----
 arch/riscv/lib/sifive_clint.c     |  6 ++----
 drivers/timer/ag101p_timer.c      |  5 ++---
 drivers/timer/altera_timer.c      |  6 ++----
 drivers/timer/arc_timer.c         |  6 ++----
 drivers/timer/ast_timer.c         |  6 ++----
 drivers/timer/atcpit100_timer.c   |  5 ++---
 drivers/timer/atmel_pit_timer.c   |  6 ++----
 drivers/timer/cadence-ttc.c       |  6 ++----
 drivers/timer/dw-apb-timer.c      |  6 ++----
 drivers/timer/mchp-pit64b-timer.c |  6 ++----
 drivers/timer/mpc83xx_timer.c     |  6 ++----
 drivers/timer/mtk_timer.c         |  6 ++----
 drivers/timer/nomadik-mtu-timer.c |  6 ++----
 drivers/timer/omap-timer.c        |  6 ++----
 drivers/timer/ostm_timer.c        |  6 ++----
 drivers/timer/riscv_timer.c       | 21 +++++++++------------
 drivers/timer/rockchip_timer.c    |  5 ++---
 drivers/timer/sandbox_timer.c     |  6 ++----
 drivers/timer/sti-timer.c         |  6 ++----
 drivers/timer/stm32_timer.c       |  6 ++----
 drivers/timer/timer-uclass.c      |  3 ++-
 drivers/timer/tsc_timer.c         |  6 ++----
 include/timer.h                   |  9 ++++++---
 24 files changed, 59 insertions(+), 97 deletions(-)

diff --git a/arch/riscv/lib/andes_plmt.c b/arch/riscv/lib/andes_plmt.c
index a28c14c1eb..cec86718c7 100644
--- a/arch/riscv/lib/andes_plmt.c
+++ b/arch/riscv/lib/andes_plmt.c
@@ -17,11 +17,9 @@
 /* mtime register */
 #define MTIME_REG(base)			((ulong)(base))
 
-static int andes_plmt_get_count(struct udevice *dev, u64 *count)
+static u64 andes_plmt_get_count(struct udevice *dev)
 {
-	*count = readq((void __iomem *)MTIME_REG(dev->priv));
-
-	return 0;
+	return readq((void __iomem *)MTIME_REG(dev->priv));
 }
 
 static const struct timer_ops andes_plmt_ops = {
diff --git a/arch/riscv/lib/sifive_clint.c b/arch/riscv/lib/sifive_clint.c
index c9704c596f..a5572cb825 100644
--- a/arch/riscv/lib/sifive_clint.c
+++ b/arch/riscv/lib/sifive_clint.c
@@ -62,11 +62,9 @@ int riscv_get_ipi(int hart, int *pending)
 	return 0;
 }
 
-static int sifive_clint_get_count(struct udevice *dev, u64 *count)
+static u64 sifive_clint_get_count(struct udevice *dev)
 {
-	*count = readq((void __iomem *)MTIME_REG(dev->priv));
-
-	return 0;
+	return readq((void __iomem *)MTIME_REG(dev->priv));
 }
 
 static const struct timer_ops sifive_clint_ops = {
diff --git a/drivers/timer/ag101p_timer.c b/drivers/timer/ag101p_timer.c
index c011906b93..23ad5b2b67 100644
--- a/drivers/timer/ag101p_timer.c
+++ b/drivers/timer/ag101p_timer.c
@@ -62,14 +62,13 @@ struct atftmr_timer_platdata {
 	struct atftmr_timer_regs *regs;
 };
 
-static int atftmr_timer_get_count(struct udevice *dev, u64 *count)
+static u64 atftmr_timer_get_count(struct udevice *dev)
 {
 	struct atftmr_timer_platdata *plat = dev->platdata;
 	struct atftmr_timer_regs *const regs = plat->regs;
 	u32 val;
 	val = readl(&regs->t3_counter);
-	*count = timer_conv_64(val);
-	return 0;
+	return timer_conv_64(val);
 }
 
 static int atftmr_timer_probe(struct udevice *dev)
diff --git a/drivers/timer/altera_timer.c b/drivers/timer/altera_timer.c
index 6cb2923e0b..ccc164ee17 100644
--- a/drivers/timer/altera_timer.c
+++ b/drivers/timer/altera_timer.c
@@ -32,7 +32,7 @@ struct altera_timer_platdata {
 	struct altera_timer_regs *regs;
 };
 
-static int altera_timer_get_count(struct udevice *dev, u64 *count)
+static u64 altera_timer_get_count(struct udevice *dev)
 {
 	struct altera_timer_platdata *plat = dev->platdata;
 	struct altera_timer_regs *const regs = plat->regs;
@@ -44,9 +44,7 @@ static int altera_timer_get_count(struct udevice *dev, u64 *count)
 	/* Read timer value */
 	val = readl(&regs->snapl) & 0xffff;
 	val |= (readl(&regs->snaph) & 0xffff) << 16;
-	*count = timer_conv_64(~val);
-
-	return 0;
+	return timer_conv_64(~val);
 }
 
 static int altera_timer_probe(struct udevice *dev)
diff --git a/drivers/timer/arc_timer.c b/drivers/timer/arc_timer.c
index 8c574ec5af..2dea9f40cb 100644
--- a/drivers/timer/arc_timer.c
+++ b/drivers/timer/arc_timer.c
@@ -26,7 +26,7 @@ struct arc_timer_priv {
 		uint timer_id;
 };
 
-static int arc_timer_get_count(struct udevice *dev, u64 *count)
+static u64 arc_timer_get_count(struct udevice *dev)
 {
 	u32 val = 0;
 	struct arc_timer_priv *priv = dev_get_priv(dev);
@@ -39,9 +39,7 @@ static int arc_timer_get_count(struct udevice *dev, u64 *count)
 		val = read_aux_reg(ARC_AUX_TIMER1_CNT);
 		break;
 	}
-	*count = timer_conv_64(val);
-
-	return 0;
+	return timer_conv_64(val);
 }
 
 static int arc_timer_probe(struct udevice *dev)
diff --git a/drivers/timer/ast_timer.c b/drivers/timer/ast_timer.c
index e313249740..35369a4087 100644
--- a/drivers/timer/ast_timer.c
+++ b/drivers/timer/ast_timer.c
@@ -51,13 +51,11 @@ static int ast_timer_probe(struct udevice *dev)
 	return 0;
 }
 
-static int ast_timer_get_count(struct udevice *dev, u64 *count)
+static u64 ast_timer_get_count(struct udevice *dev)
 {
 	struct ast_timer_priv *priv = dev_get_priv(dev);
 
-	*count = AST_TMC_RELOAD_VAL - readl(&priv->tmc->status);
-
-	return 0;
+	return AST_TMC_RELOAD_VAL - readl(&priv->tmc->status);
 }
 
 static int ast_timer_ofdata_to_platdata(struct udevice *dev)
diff --git a/drivers/timer/atcpit100_timer.c b/drivers/timer/atcpit100_timer.c
index 5d4ae68509..fcb8a45358 100644
--- a/drivers/timer/atcpit100_timer.c
+++ b/drivers/timer/atcpit100_timer.c
@@ -68,13 +68,12 @@ struct atcpit_timer_platdata {
 	u32 *regs;
 };
 
-static int atcpit_timer_get_count(struct udevice *dev, u64 *count)
+static u64 atcpit_timer_get_count(struct udevice *dev)
 {
 	struct atcpit_timer_platdata *plat = dev_get_platdata(dev);
 	u32 val;
 	val = ~(REG32_TMR(CH_CNT(1))+0xffffffff);
-	*count = timer_conv_64(val);
-	return 0;
+	return timer_conv_64(val);
 }
 
 static int atcpit_timer_probe(struct udevice *dev)
diff --git a/drivers/timer/atmel_pit_timer.c b/drivers/timer/atmel_pit_timer.c
index 843d670b5e..9f0ad1d703 100644
--- a/drivers/timer/atmel_pit_timer.c
+++ b/drivers/timer/atmel_pit_timer.c
@@ -25,15 +25,13 @@ struct atmel_pit_platdata {
 	struct atmel_pit_regs *regs;
 };
 
-static int atmel_pit_get_count(struct udevice *dev, u64 *count)
+static u64 atmel_pit_get_count(struct udevice *dev)
 {
 	struct atmel_pit_platdata *plat = dev_get_platdata(dev);
 	struct atmel_pit_regs *const regs = plat->regs;
 	u32 val = readl(&regs->value_image);
 
-	*count = timer_conv_64(val);
-
-	return 0;
+	return timer_conv_64(val);
 }
 
 static int atmel_pit_probe(struct udevice *dev)
diff --git a/drivers/timer/cadence-ttc.c b/drivers/timer/cadence-ttc.c
index e6b6dfe376..bebb2c2e90 100644
--- a/drivers/timer/cadence-ttc.c
+++ b/drivers/timer/cadence-ttc.c
@@ -57,13 +57,11 @@ ulong timer_get_boot_us(void)
 }
 #endif
 
-static int cadence_ttc_get_count(struct udevice *dev, u64 *count)
+static u64 cadence_ttc_get_count(struct udevice *dev)
 {
 	struct cadence_ttc_priv *priv = dev_get_priv(dev);
 
-	*count = readl(&priv->regs->counter_val1);
-
-	return 0;
+	return readl(&priv->regs->counter_val1);
 }
 
 static int cadence_ttc_probe(struct udevice *dev)
diff --git a/drivers/timer/dw-apb-timer.c b/drivers/timer/dw-apb-timer.c
index 35271b20c8..68bc258131 100644
--- a/drivers/timer/dw-apb-timer.c
+++ b/drivers/timer/dw-apb-timer.c
@@ -25,7 +25,7 @@ struct dw_apb_timer_priv {
 	struct reset_ctl_bulk resets;
 };
 
-static int dw_apb_timer_get_count(struct udevice *dev, u64 *count)
+static u64 dw_apb_timer_get_count(struct udevice *dev)
 {
 	struct dw_apb_timer_priv *priv = dev_get_priv(dev);
 
@@ -34,9 +34,7 @@ static int dw_apb_timer_get_count(struct udevice *dev, u64 *count)
 	 * requires the count to be incrementing. Invert the
 	 * result.
 	 */
-	*count = timer_conv_64(~readl(priv->regs + DW_APB_CURR_VAL));
-
-	return 0;
+	return timer_conv_64(~readl(priv->regs + DW_APB_CURR_VAL));
 }
 
 static int dw_apb_timer_probe(struct udevice *dev)
diff --git a/drivers/timer/mchp-pit64b-timer.c b/drivers/timer/mchp-pit64b-timer.c
index ead8c9b84a..ad962098b3 100644
--- a/drivers/timer/mchp-pit64b-timer.c
+++ b/drivers/timer/mchp-pit64b-timer.c
@@ -27,16 +27,14 @@ struct mchp_pit64b_priv {
 	void __iomem *base;
 };
 
-static int mchp_pit64b_get_count(struct udevice *dev, u64 *count)
+static u64 mchp_pit64b_get_count(struct udevice *dev)
 {
 	struct mchp_pit64b_priv *priv = dev_get_priv(dev);
 
 	u32 lsb = readl(priv->base + MCHP_PIT64B_TLSBR);
 	u32 msb = readl(priv->base + MCHP_PIT64B_TMSBR);
 
-	*count = ((u64)msb << 32) | lsb;
-
-	return 0;
+	return ((u64)msb << 32) | lsb;
 }
 
 static int mchp_pit64b_probe(struct udevice *dev)
diff --git a/drivers/timer/mpc83xx_timer.c b/drivers/timer/mpc83xx_timer.c
index ad8bb28e8b..ba7704225a 100644
--- a/drivers/timer/mpc83xx_timer.c
+++ b/drivers/timer/mpc83xx_timer.c
@@ -187,7 +187,7 @@ void wait_ticks(ulong ticks)
 		WATCHDOG_RESET();
 }
 
-static int mpc83xx_timer_get_count(struct udevice *dev, u64 *count)
+static u64 mpc83xx_timer_get_count(struct udevice *dev)
 {
 	u32 tbu, tbl;
 
@@ -201,9 +201,7 @@ static int mpc83xx_timer_get_count(struct udevice *dev, u64 *count)
 		tbl = mftb();
 	} while (tbu != mftbu());
 
-	*count = (tbu * 0x10000ULL) + tbl;
-
-	return 0;
+	return (tbu * 0x10000ULL) + tbl;
 }
 
 static int mpc83xx_timer_probe(struct udevice *dev)
diff --git a/drivers/timer/mtk_timer.c b/drivers/timer/mtk_timer.c
index 69ed521811..74e9ea34ff 100644
--- a/drivers/timer/mtk_timer.c
+++ b/drivers/timer/mtk_timer.c
@@ -27,14 +27,12 @@ struct mtk_timer_priv {
 	void __iomem *base;
 };
 
-static int mtk_timer_get_count(struct udevice *dev, u64 *count)
+static u64 mtk_timer_get_count(struct udevice *dev)
 {
 	struct mtk_timer_priv *priv = dev_get_priv(dev);
 	u32 val = readl(priv->base + MTK_GPT4_CNT);
 
-	*count = timer_conv_64(val);
-
-	return 0;
+	return timer_conv_64(val);
 }
 
 static int mtk_timer_probe(struct udevice *dev)
diff --git a/drivers/timer/nomadik-mtu-timer.c b/drivers/timer/nomadik-mtu-timer.c
index 7ff921385a..d7f7ca4eff 100644
--- a/drivers/timer/nomadik-mtu-timer.c
+++ b/drivers/timer/nomadik-mtu-timer.c
@@ -54,14 +54,12 @@ struct nomadik_mtu_priv {
 	struct nomadik_mtu_timer_regs *timer;
 };
 
-static int nomadik_mtu_get_count(struct udevice *dev, u64 *count)
+static u64 nomadik_mtu_get_count(struct udevice *dev)
 {
 	struct nomadik_mtu_priv *priv = dev_get_priv(dev);
 
 	/* Decrementing counter: invert the value */
-	*count = timer_conv_64(~readl(&priv->timer->cv));
-
-	return 0;
+	return timer_conv_64(~readl(&priv->timer->cv));
 }
 
 static int nomadik_mtu_probe(struct udevice *dev)
diff --git a/drivers/timer/omap-timer.c b/drivers/timer/omap-timer.c
index cf3d27b96b..4eecb3e64d 100644
--- a/drivers/timer/omap-timer.c
+++ b/drivers/timer/omap-timer.c
@@ -48,13 +48,11 @@ struct omap_timer_priv {
 	struct omap_gptimer_regs *regs;
 };
 
-static int omap_timer_get_count(struct udevice *dev, u64 *count)
+static u64 omap_timer_get_count(struct udevice *dev)
 {
 	struct omap_timer_priv *priv = dev_get_priv(dev);
 
-	*count = timer_conv_64(readl(&priv->regs->tcrr));
-
-	return 0;
+	return timer_conv_64(readl(&priv->regs->tcrr));
 }
 
 static int omap_timer_probe(struct udevice *dev)
diff --git a/drivers/timer/ostm_timer.c b/drivers/timer/ostm_timer.c
index bea97159eb..bb0636a071 100644
--- a/drivers/timer/ostm_timer.c
+++ b/drivers/timer/ostm_timer.c
@@ -27,13 +27,11 @@ struct ostm_priv {
 	fdt_addr_t	regs;
 };
 
-static int ostm_get_count(struct udevice *dev, u64 *count)
+static u64 ostm_get_count(struct udevice *dev)
 {
 	struct ostm_priv *priv = dev_get_priv(dev);
 
-	*count = timer_conv_64(readl(priv->regs + OSTM_CNT));
-
-	return 0;
+	return timer_conv_64(readl(priv->regs + OSTM_CNT));
 }
 
 static int ostm_probe(struct udevice *dev)
diff --git a/drivers/timer/riscv_timer.c b/drivers/timer/riscv_timer.c
index 449fcfcfd5..21ae184057 100644
--- a/drivers/timer/riscv_timer.c
+++ b/drivers/timer/riscv_timer.c
@@ -16,22 +16,19 @@
 #include <timer.h>
 #include <asm/csr.h>
 
-static int riscv_timer_get_count(struct udevice *dev, u64 *count)
+static u64 riscv_timer_get_count(struct udevice *dev)
 {
-	if (IS_ENABLED(CONFIG_64BIT)) {
-		*count = csr_read(CSR_TIME);
-	} else {
-		u32 hi, lo;
+	__maybe_unused u32 hi, lo;
 
-		do {
-			hi = csr_read(CSR_TIMEH);
-			lo = csr_read(CSR_TIME);
-		} while (hi != csr_read(CSR_TIMEH));
+	if (IS_ENABLED(CONFIG_64BIT))
+		return csr_read(CSR_TIME);
 
-		*count = ((u64)hi << 32) | lo;
-	}
+	do {
+		hi = csr_read(CSR_TIMEH);
+		lo = csr_read(CSR_TIME);
+	} while (hi != csr_read(CSR_TIMEH));
 
-	return 0;
+	return ((u64)hi << 32) | lo;
 }
 
 static int riscv_timer_probe(struct udevice *dev)
diff --git a/drivers/timer/rockchip_timer.c b/drivers/timer/rockchip_timer.c
index 7a5a484252..53cdf09810 100644
--- a/drivers/timer/rockchip_timer.c
+++ b/drivers/timer/rockchip_timer.c
@@ -88,14 +88,13 @@ ulong timer_get_boot_us(void)
 }
 #endif
 
-static int rockchip_timer_get_count(struct udevice *dev, u64 *count)
+static u64 rockchip_timer_get_count(struct udevice *dev)
 {
 	struct rockchip_timer_priv *priv = dev_get_priv(dev);
 	uint64_t cntr = rockchip_timer_get_curr_value(priv->timer);
 
 	/* timers are down-counting */
-	*count = ~0ull - cntr;
-	return 0;
+	return ~0ull - cntr;
 }
 
 static int rockchip_clk_ofdata_to_platdata(struct udevice *dev)
diff --git a/drivers/timer/sandbox_timer.c b/drivers/timer/sandbox_timer.c
index 6a503c2f15..135c0f38a4 100644
--- a/drivers/timer/sandbox_timer.c
+++ b/drivers/timer/sandbox_timer.c
@@ -29,11 +29,9 @@ unsigned long notrace timer_early_get_rate(void)
 	return SANDBOX_TIMER_RATE;
 }
 
-static notrace int sandbox_timer_get_count(struct udevice *dev, u64 *count)
+static notrace u64 sandbox_timer_get_count(struct udevice *dev)
 {
-	*count = timer_early_get_count();
-
-	return 0;
+	return timer_early_get_count();
 }
 
 static int sandbox_timer_probe(struct udevice *dev)
diff --git a/drivers/timer/sti-timer.c b/drivers/timer/sti-timer.c
index ff42056abd..e6843ebb33 100644
--- a/drivers/timer/sti-timer.c
+++ b/drivers/timer/sti-timer.c
@@ -17,7 +17,7 @@ struct sti_timer_priv {
 	struct globaltimer *global_timer;
 };
 
-static int sti_timer_get_count(struct udevice *dev, u64 *count)
+static u64 sti_timer_get_count(struct udevice *dev)
 {
 	struct sti_timer_priv *priv = dev_get_priv(dev);
 	struct globaltimer *global_timer = priv->global_timer;
@@ -34,9 +34,7 @@ static int sti_timer_get_count(struct udevice *dev, u64 *count)
 			old = high;
 	}
 	timer = high;
-	*count = (u64)((timer << 32) | low);
-
-	return 0;
+	return (u64)((timer << 32) | low);
 }
 
 static int sti_timer_probe(struct udevice *dev)
diff --git a/drivers/timer/stm32_timer.c b/drivers/timer/stm32_timer.c
index c57fa3f557..f517d5e61f 100644
--- a/drivers/timer/stm32_timer.c
+++ b/drivers/timer/stm32_timer.c
@@ -52,14 +52,12 @@ struct stm32_timer_priv {
 	struct stm32_timer_regs *base;
 };
 
-static int stm32_timer_get_count(struct udevice *dev, u64 *count)
+static u64 stm32_timer_get_count(struct udevice *dev)
 {
 	struct stm32_timer_priv *priv = dev_get_priv(dev);
 	struct stm32_timer_regs *regs = priv->base;
 
-	*count = readl(&regs->cnt);
-
-	return 0;
+	return readl(&regs->cnt);
 }
 
 static int stm32_timer_probe(struct udevice *dev)
diff --git a/drivers/timer/timer-uclass.c b/drivers/timer/timer-uclass.c
index e9802c8b43..53a8dfb316 100644
--- a/drivers/timer/timer-uclass.c
+++ b/drivers/timer/timer-uclass.c
@@ -33,7 +33,8 @@ int notrace timer_get_count(struct udevice *dev, u64 *count)
 	if (!ops->get_count)
 		return -ENOSYS;
 
-	return ops->get_count(dev, count);
+	*count = ops->get_count(dev);
+	return 0;
 }
 
 unsigned long notrace timer_get_rate(struct udevice *dev)
diff --git a/drivers/timer/tsc_timer.c b/drivers/timer/tsc_timer.c
index 93c959ff44..abc0a1da05 100644
--- a/drivers/timer/tsc_timer.c
+++ b/drivers/timer/tsc_timer.c
@@ -386,13 +386,11 @@ void __udelay(unsigned long usec)
 #endif
 }
 
-static int tsc_timer_get_count(struct udevice *dev, u64 *count)
+static u64 tsc_timer_get_count(struct udevice *dev)
 {
 	u64 now_tick = rdtsc();
 
-	*count = now_tick - gd->arch.tsc_base;
-
-	return 0;
+	return now_tick - gd->arch.tsc_base;
 }
 
 static void tsc_timer_ensure_setup(bool early)
diff --git a/include/timer.h b/include/timer.h
index aa9d870619..a044cb034e 100644
--- a/include/timer.h
+++ b/include/timer.h
@@ -67,11 +67,14 @@ struct timer_ops {
 	 *
 	 * @dev: The timer device
 	 *
-	 * @count: pointer that returns the current 64-bit timer count
+	 * This function may be called at any time after the driver is probed.
+	 * All necessary initialization must be completed by the time probe()
+	 * returns. The count returned by this functions should be monotonic.
+	 * This function must succeed.
 	 *
-	 * Return: 0 if OK, -ve on error
+	 * Return: The current 64-bit timer count
 	 */
-	int (*get_count)(struct udevice *dev, u64 *count);
+	u64 (*get_count)(struct udevice *dev);
 };
 
 /**
-- 
2.28.0

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 2/2] timer: Return count from timer_ops.get_count
  2020-10-07 18:37 ` [PATCH v2 2/2] timer: Return count from timer_ops.get_count Sean Anderson
@ 2020-10-08  8:28   ` Claudiu.Beznea at microchip.com
  2020-10-09 17:23   ` Simon Glass
  2020-10-23  0:27   ` Tom Rini
  2 siblings, 0 replies; 8+ messages in thread
From: Claudiu.Beznea at microchip.com @ 2020-10-08  8:28 UTC (permalink / raw)
  To: u-boot



On 07.10.2020 21:37, Sean Anderson wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> No timer drivers return an error from get_count. Instead of possibly
> returning an error, just return the count directly.
> 
> Signed-off-by: Sean Anderson <seanga2@gmail.com>

For mchp-pit64b-timer:
Reviewed-by: Claudiu Beznea <claudiu.beznea@microchip.com>

> ---
> 
> Changes in v2:
> - mchp-pit64b was added since v1, so convert it
> - Document when get_count may be called, and what assumptions the timer
>   subsystem makes about drivers
> 
>  arch/riscv/lib/andes_plmt.c       |  6 ++----
>  arch/riscv/lib/sifive_clint.c     |  6 ++----
>  drivers/timer/ag101p_timer.c      |  5 ++---
>  drivers/timer/altera_timer.c      |  6 ++----
>  drivers/timer/arc_timer.c         |  6 ++----
>  drivers/timer/ast_timer.c         |  6 ++----
>  drivers/timer/atcpit100_timer.c   |  5 ++---
>  drivers/timer/atmel_pit_timer.c   |  6 ++----
>  drivers/timer/cadence-ttc.c       |  6 ++----
>  drivers/timer/dw-apb-timer.c      |  6 ++----
>  drivers/timer/mchp-pit64b-timer.c |  6 ++----
>  drivers/timer/mpc83xx_timer.c     |  6 ++----
>  drivers/timer/mtk_timer.c         |  6 ++----
>  drivers/timer/nomadik-mtu-timer.c |  6 ++----
>  drivers/timer/omap-timer.c        |  6 ++----
>  drivers/timer/ostm_timer.c        |  6 ++----
>  drivers/timer/riscv_timer.c       | 21 +++++++++------------
>  drivers/timer/rockchip_timer.c    |  5 ++---
>  drivers/timer/sandbox_timer.c     |  6 ++----
>  drivers/timer/sti-timer.c         |  6 ++----
>  drivers/timer/stm32_timer.c       |  6 ++----
>  drivers/timer/timer-uclass.c      |  3 ++-
>  drivers/timer/tsc_timer.c         |  6 ++----
>  include/timer.h                   |  9 ++++++---
>  24 files changed, 59 insertions(+), 97 deletions(-)
> 
> diff --git a/arch/riscv/lib/andes_plmt.c b/arch/riscv/lib/andes_plmt.c
> index a28c14c1eb..cec86718c7 100644
> --- a/arch/riscv/lib/andes_plmt.c
> +++ b/arch/riscv/lib/andes_plmt.c
> @@ -17,11 +17,9 @@
>  /* mtime register */
>  #define MTIME_REG(base)                        ((ulong)(base))
> 
> -static int andes_plmt_get_count(struct udevice *dev, u64 *count)
> +static u64 andes_plmt_get_count(struct udevice *dev)
>  {
> -       *count = readq((void __iomem *)MTIME_REG(dev->priv));
> -
> -       return 0;
> +       return readq((void __iomem *)MTIME_REG(dev->priv));
>  }
> 
>  static const struct timer_ops andes_plmt_ops = {
> diff --git a/arch/riscv/lib/sifive_clint.c b/arch/riscv/lib/sifive_clint.c
> index c9704c596f..a5572cb825 100644
> --- a/arch/riscv/lib/sifive_clint.c
> +++ b/arch/riscv/lib/sifive_clint.c
> @@ -62,11 +62,9 @@ int riscv_get_ipi(int hart, int *pending)
>         return 0;
>  }
> 
> -static int sifive_clint_get_count(struct udevice *dev, u64 *count)
> +static u64 sifive_clint_get_count(struct udevice *dev)
>  {
> -       *count = readq((void __iomem *)MTIME_REG(dev->priv));
> -
> -       return 0;
> +       return readq((void __iomem *)MTIME_REG(dev->priv));
>  }
> 
>  static const struct timer_ops sifive_clint_ops = {
> diff --git a/drivers/timer/ag101p_timer.c b/drivers/timer/ag101p_timer.c
> index c011906b93..23ad5b2b67 100644
> --- a/drivers/timer/ag101p_timer.c
> +++ b/drivers/timer/ag101p_timer.c
> @@ -62,14 +62,13 @@ struct atftmr_timer_platdata {
>         struct atftmr_timer_regs *regs;
>  };
> 
> -static int atftmr_timer_get_count(struct udevice *dev, u64 *count)
> +static u64 atftmr_timer_get_count(struct udevice *dev)
>  {
>         struct atftmr_timer_platdata *plat = dev->platdata;
>         struct atftmr_timer_regs *const regs = plat->regs;
>         u32 val;
>         val = readl(&regs->t3_counter);
> -       *count = timer_conv_64(val);
> -       return 0;
> +       return timer_conv_64(val);
>  }
> 
>  static int atftmr_timer_probe(struct udevice *dev)
> diff --git a/drivers/timer/altera_timer.c b/drivers/timer/altera_timer.c
> index 6cb2923e0b..ccc164ee17 100644
> --- a/drivers/timer/altera_timer.c
> +++ b/drivers/timer/altera_timer.c
> @@ -32,7 +32,7 @@ struct altera_timer_platdata {
>         struct altera_timer_regs *regs;
>  };
> 
> -static int altera_timer_get_count(struct udevice *dev, u64 *count)
> +static u64 altera_timer_get_count(struct udevice *dev)
>  {
>         struct altera_timer_platdata *plat = dev->platdata;
>         struct altera_timer_regs *const regs = plat->regs;
> @@ -44,9 +44,7 @@ static int altera_timer_get_count(struct udevice *dev, u64 *count)
>         /* Read timer value */
>         val = readl(&regs->snapl) & 0xffff;
>         val |= (readl(&regs->snaph) & 0xffff) << 16;
> -       *count = timer_conv_64(~val);
> -
> -       return 0;
> +       return timer_conv_64(~val);
>  }
> 
>  static int altera_timer_probe(struct udevice *dev)
> diff --git a/drivers/timer/arc_timer.c b/drivers/timer/arc_timer.c
> index 8c574ec5af..2dea9f40cb 100644
> --- a/drivers/timer/arc_timer.c
> +++ b/drivers/timer/arc_timer.c
> @@ -26,7 +26,7 @@ struct arc_timer_priv {
>                 uint timer_id;
>  };
> 
> -static int arc_timer_get_count(struct udevice *dev, u64 *count)
> +static u64 arc_timer_get_count(struct udevice *dev)
>  {
>         u32 val = 0;
>         struct arc_timer_priv *priv = dev_get_priv(dev);
> @@ -39,9 +39,7 @@ static int arc_timer_get_count(struct udevice *dev, u64 *count)
>                 val = read_aux_reg(ARC_AUX_TIMER1_CNT);
>                 break;
>         }
> -       *count = timer_conv_64(val);
> -
> -       return 0;
> +       return timer_conv_64(val);
>  }
> 
>  static int arc_timer_probe(struct udevice *dev)
> diff --git a/drivers/timer/ast_timer.c b/drivers/timer/ast_timer.c
> index e313249740..35369a4087 100644
> --- a/drivers/timer/ast_timer.c
> +++ b/drivers/timer/ast_timer.c
> @@ -51,13 +51,11 @@ static int ast_timer_probe(struct udevice *dev)
>         return 0;
>  }
> 
> -static int ast_timer_get_count(struct udevice *dev, u64 *count)
> +static u64 ast_timer_get_count(struct udevice *dev)
>  {
>         struct ast_timer_priv *priv = dev_get_priv(dev);
> 
> -       *count = AST_TMC_RELOAD_VAL - readl(&priv->tmc->status);
> -
> -       return 0;
> +       return AST_TMC_RELOAD_VAL - readl(&priv->tmc->status);
>  }
> 
>  static int ast_timer_ofdata_to_platdata(struct udevice *dev)
> diff --git a/drivers/timer/atcpit100_timer.c b/drivers/timer/atcpit100_timer.c
> index 5d4ae68509..fcb8a45358 100644
> --- a/drivers/timer/atcpit100_timer.c
> +++ b/drivers/timer/atcpit100_timer.c
> @@ -68,13 +68,12 @@ struct atcpit_timer_platdata {
>         u32 *regs;
>  };
> 
> -static int atcpit_timer_get_count(struct udevice *dev, u64 *count)
> +static u64 atcpit_timer_get_count(struct udevice *dev)
>  {
>         struct atcpit_timer_platdata *plat = dev_get_platdata(dev);
>         u32 val;
>         val = ~(REG32_TMR(CH_CNT(1))+0xffffffff);
> -       *count = timer_conv_64(val);
> -       return 0;
> +       return timer_conv_64(val);
>  }
> 
>  static int atcpit_timer_probe(struct udevice *dev)
> diff --git a/drivers/timer/atmel_pit_timer.c b/drivers/timer/atmel_pit_timer.c
> index 843d670b5e..9f0ad1d703 100644
> --- a/drivers/timer/atmel_pit_timer.c
> +++ b/drivers/timer/atmel_pit_timer.c
> @@ -25,15 +25,13 @@ struct atmel_pit_platdata {
>         struct atmel_pit_regs *regs;
>  };
> 
> -static int atmel_pit_get_count(struct udevice *dev, u64 *count)
> +static u64 atmel_pit_get_count(struct udevice *dev)
>  {
>         struct atmel_pit_platdata *plat = dev_get_platdata(dev);
>         struct atmel_pit_regs *const regs = plat->regs;
>         u32 val = readl(&regs->value_image);
> 
> -       *count = timer_conv_64(val);
> -
> -       return 0;
> +       return timer_conv_64(val);
>  }
> 
>  static int atmel_pit_probe(struct udevice *dev)
> diff --git a/drivers/timer/cadence-ttc.c b/drivers/timer/cadence-ttc.c
> index e6b6dfe376..bebb2c2e90 100644
> --- a/drivers/timer/cadence-ttc.c
> +++ b/drivers/timer/cadence-ttc.c
> @@ -57,13 +57,11 @@ ulong timer_get_boot_us(void)
>  }
>  #endif
> 
> -static int cadence_ttc_get_count(struct udevice *dev, u64 *count)
> +static u64 cadence_ttc_get_count(struct udevice *dev)
>  {
>         struct cadence_ttc_priv *priv = dev_get_priv(dev);
> 
> -       *count = readl(&priv->regs->counter_val1);
> -
> -       return 0;
> +       return readl(&priv->regs->counter_val1);
>  }
> 
>  static int cadence_ttc_probe(struct udevice *dev)
> diff --git a/drivers/timer/dw-apb-timer.c b/drivers/timer/dw-apb-timer.c
> index 35271b20c8..68bc258131 100644
> --- a/drivers/timer/dw-apb-timer.c
> +++ b/drivers/timer/dw-apb-timer.c
> @@ -25,7 +25,7 @@ struct dw_apb_timer_priv {
>         struct reset_ctl_bulk resets;
>  };
> 
> -static int dw_apb_timer_get_count(struct udevice *dev, u64 *count)
> +static u64 dw_apb_timer_get_count(struct udevice *dev)
>  {
>         struct dw_apb_timer_priv *priv = dev_get_priv(dev);
> 
> @@ -34,9 +34,7 @@ static int dw_apb_timer_get_count(struct udevice *dev, u64 *count)
>          * requires the count to be incrementing. Invert the
>          * result.
>          */
> -       *count = timer_conv_64(~readl(priv->regs + DW_APB_CURR_VAL));
> -
> -       return 0;
> +       return timer_conv_64(~readl(priv->regs + DW_APB_CURR_VAL));
>  }
> 
>  static int dw_apb_timer_probe(struct udevice *dev)
> diff --git a/drivers/timer/mchp-pit64b-timer.c b/drivers/timer/mchp-pit64b-timer.c
> index ead8c9b84a..ad962098b3 100644
> --- a/drivers/timer/mchp-pit64b-timer.c
> +++ b/drivers/timer/mchp-pit64b-timer.c
> @@ -27,16 +27,14 @@ struct mchp_pit64b_priv {
>         void __iomem *base;
>  };
> 
> -static int mchp_pit64b_get_count(struct udevice *dev, u64 *count)
> +static u64 mchp_pit64b_get_count(struct udevice *dev)
>  {
>         struct mchp_pit64b_priv *priv = dev_get_priv(dev);
> 
>         u32 lsb = readl(priv->base + MCHP_PIT64B_TLSBR);
>         u32 msb = readl(priv->base + MCHP_PIT64B_TMSBR);
> 
> -       *count = ((u64)msb << 32) | lsb;
> -
> -       return 0;
> +       return ((u64)msb << 32) | lsb;
>  }
> 
>  static int mchp_pit64b_probe(struct udevice *dev)
> diff --git a/drivers/timer/mpc83xx_timer.c b/drivers/timer/mpc83xx_timer.c
> index ad8bb28e8b..ba7704225a 100644
> --- a/drivers/timer/mpc83xx_timer.c
> +++ b/drivers/timer/mpc83xx_timer.c
> @@ -187,7 +187,7 @@ void wait_ticks(ulong ticks)
>                 WATCHDOG_RESET();
>  }
> 
> -static int mpc83xx_timer_get_count(struct udevice *dev, u64 *count)
> +static u64 mpc83xx_timer_get_count(struct udevice *dev)
>  {
>         u32 tbu, tbl;
> 
> @@ -201,9 +201,7 @@ static int mpc83xx_timer_get_count(struct udevice *dev, u64 *count)
>                 tbl = mftb();
>         } while (tbu != mftbu());
> 
> -       *count = (tbu * 0x10000ULL) + tbl;
> -
> -       return 0;
> +       return (tbu * 0x10000ULL) + tbl;
>  }
> 
>  static int mpc83xx_timer_probe(struct udevice *dev)
> diff --git a/drivers/timer/mtk_timer.c b/drivers/timer/mtk_timer.c
> index 69ed521811..74e9ea34ff 100644
> --- a/drivers/timer/mtk_timer.c
> +++ b/drivers/timer/mtk_timer.c
> @@ -27,14 +27,12 @@ struct mtk_timer_priv {
>         void __iomem *base;
>  };
> 
> -static int mtk_timer_get_count(struct udevice *dev, u64 *count)
> +static u64 mtk_timer_get_count(struct udevice *dev)
>  {
>         struct mtk_timer_priv *priv = dev_get_priv(dev);
>         u32 val = readl(priv->base + MTK_GPT4_CNT);
> 
> -       *count = timer_conv_64(val);
> -
> -       return 0;
> +       return timer_conv_64(val);
>  }
> 
>  static int mtk_timer_probe(struct udevice *dev)
> diff --git a/drivers/timer/nomadik-mtu-timer.c b/drivers/timer/nomadik-mtu-timer.c
> index 7ff921385a..d7f7ca4eff 100644
> --- a/drivers/timer/nomadik-mtu-timer.c
> +++ b/drivers/timer/nomadik-mtu-timer.c
> @@ -54,14 +54,12 @@ struct nomadik_mtu_priv {
>         struct nomadik_mtu_timer_regs *timer;
>  };
> 
> -static int nomadik_mtu_get_count(struct udevice *dev, u64 *count)
> +static u64 nomadik_mtu_get_count(struct udevice *dev)
>  {
>         struct nomadik_mtu_priv *priv = dev_get_priv(dev);
> 
>         /* Decrementing counter: invert the value */
> -       *count = timer_conv_64(~readl(&priv->timer->cv));
> -
> -       return 0;
> +       return timer_conv_64(~readl(&priv->timer->cv));
>  }
> 
>  static int nomadik_mtu_probe(struct udevice *dev)
> diff --git a/drivers/timer/omap-timer.c b/drivers/timer/omap-timer.c
> index cf3d27b96b..4eecb3e64d 100644
> --- a/drivers/timer/omap-timer.c
> +++ b/drivers/timer/omap-timer.c
> @@ -48,13 +48,11 @@ struct omap_timer_priv {
>         struct omap_gptimer_regs *regs;
>  };
> 
> -static int omap_timer_get_count(struct udevice *dev, u64 *count)
> +static u64 omap_timer_get_count(struct udevice *dev)
>  {
>         struct omap_timer_priv *priv = dev_get_priv(dev);
> 
> -       *count = timer_conv_64(readl(&priv->regs->tcrr));
> -
> -       return 0;
> +       return timer_conv_64(readl(&priv->regs->tcrr));
>  }
> 
>  static int omap_timer_probe(struct udevice *dev)
> diff --git a/drivers/timer/ostm_timer.c b/drivers/timer/ostm_timer.c
> index bea97159eb..bb0636a071 100644
> --- a/drivers/timer/ostm_timer.c
> +++ b/drivers/timer/ostm_timer.c
> @@ -27,13 +27,11 @@ struct ostm_priv {
>         fdt_addr_t      regs;
>  };
> 
> -static int ostm_get_count(struct udevice *dev, u64 *count)
> +static u64 ostm_get_count(struct udevice *dev)
>  {
>         struct ostm_priv *priv = dev_get_priv(dev);
> 
> -       *count = timer_conv_64(readl(priv->regs + OSTM_CNT));
> -
> -       return 0;
> +       return timer_conv_64(readl(priv->regs + OSTM_CNT));
>  }
> 
>  static int ostm_probe(struct udevice *dev)
> diff --git a/drivers/timer/riscv_timer.c b/drivers/timer/riscv_timer.c
> index 449fcfcfd5..21ae184057 100644
> --- a/drivers/timer/riscv_timer.c
> +++ b/drivers/timer/riscv_timer.c
> @@ -16,22 +16,19 @@
>  #include <timer.h>
>  #include <asm/csr.h>
> 
> -static int riscv_timer_get_count(struct udevice *dev, u64 *count)
> +static u64 riscv_timer_get_count(struct udevice *dev)
>  {
> -       if (IS_ENABLED(CONFIG_64BIT)) {
> -               *count = csr_read(CSR_TIME);
> -       } else {
> -               u32 hi, lo;
> +       __maybe_unused u32 hi, lo;
> 
> -               do {
> -                       hi = csr_read(CSR_TIMEH);
> -                       lo = csr_read(CSR_TIME);
> -               } while (hi != csr_read(CSR_TIMEH));
> +       if (IS_ENABLED(CONFIG_64BIT))
> +               return csr_read(CSR_TIME);
> 
> -               *count = ((u64)hi << 32) | lo;
> -       }
> +       do {
> +               hi = csr_read(CSR_TIMEH);
> +               lo = csr_read(CSR_TIME);
> +       } while (hi != csr_read(CSR_TIMEH));
> 
> -       return 0;
> +       return ((u64)hi << 32) | lo;
>  }
> 
>  static int riscv_timer_probe(struct udevice *dev)
> diff --git a/drivers/timer/rockchip_timer.c b/drivers/timer/rockchip_timer.c
> index 7a5a484252..53cdf09810 100644
> --- a/drivers/timer/rockchip_timer.c
> +++ b/drivers/timer/rockchip_timer.c
> @@ -88,14 +88,13 @@ ulong timer_get_boot_us(void)
>  }
>  #endif
> 
> -static int rockchip_timer_get_count(struct udevice *dev, u64 *count)
> +static u64 rockchip_timer_get_count(struct udevice *dev)
>  {
>         struct rockchip_timer_priv *priv = dev_get_priv(dev);
>         uint64_t cntr = rockchip_timer_get_curr_value(priv->timer);
> 
>         /* timers are down-counting */
> -       *count = ~0ull - cntr;
> -       return 0;
> +       return ~0ull - cntr;
>  }
> 
>  static int rockchip_clk_ofdata_to_platdata(struct udevice *dev)
> diff --git a/drivers/timer/sandbox_timer.c b/drivers/timer/sandbox_timer.c
> index 6a503c2f15..135c0f38a4 100644
> --- a/drivers/timer/sandbox_timer.c
> +++ b/drivers/timer/sandbox_timer.c
> @@ -29,11 +29,9 @@ unsigned long notrace timer_early_get_rate(void)
>         return SANDBOX_TIMER_RATE;
>  }
> 
> -static notrace int sandbox_timer_get_count(struct udevice *dev, u64 *count)
> +static notrace u64 sandbox_timer_get_count(struct udevice *dev)
>  {
> -       *count = timer_early_get_count();
> -
> -       return 0;
> +       return timer_early_get_count();
>  }
> 
>  static int sandbox_timer_probe(struct udevice *dev)
> diff --git a/drivers/timer/sti-timer.c b/drivers/timer/sti-timer.c
> index ff42056abd..e6843ebb33 100644
> --- a/drivers/timer/sti-timer.c
> +++ b/drivers/timer/sti-timer.c
> @@ -17,7 +17,7 @@ struct sti_timer_priv {
>         struct globaltimer *global_timer;
>  };
> 
> -static int sti_timer_get_count(struct udevice *dev, u64 *count)
> +static u64 sti_timer_get_count(struct udevice *dev)
>  {
>         struct sti_timer_priv *priv = dev_get_priv(dev);
>         struct globaltimer *global_timer = priv->global_timer;
> @@ -34,9 +34,7 @@ static int sti_timer_get_count(struct udevice *dev, u64 *count)
>                         old = high;
>         }
>         timer = high;
> -       *count = (u64)((timer << 32) | low);
> -
> -       return 0;
> +       return (u64)((timer << 32) | low);
>  }
> 
>  static int sti_timer_probe(struct udevice *dev)
> diff --git a/drivers/timer/stm32_timer.c b/drivers/timer/stm32_timer.c
> index c57fa3f557..f517d5e61f 100644
> --- a/drivers/timer/stm32_timer.c
> +++ b/drivers/timer/stm32_timer.c
> @@ -52,14 +52,12 @@ struct stm32_timer_priv {
>         struct stm32_timer_regs *base;
>  };
> 
> -static int stm32_timer_get_count(struct udevice *dev, u64 *count)
> +static u64 stm32_timer_get_count(struct udevice *dev)
>  {
>         struct stm32_timer_priv *priv = dev_get_priv(dev);
>         struct stm32_timer_regs *regs = priv->base;
> 
> -       *count = readl(&regs->cnt);
> -
> -       return 0;
> +       return readl(&regs->cnt);
>  }
> 
>  static int stm32_timer_probe(struct udevice *dev)
> diff --git a/drivers/timer/timer-uclass.c b/drivers/timer/timer-uclass.c
> index e9802c8b43..53a8dfb316 100644
> --- a/drivers/timer/timer-uclass.c
> +++ b/drivers/timer/timer-uclass.c
> @@ -33,7 +33,8 @@ int notrace timer_get_count(struct udevice *dev, u64 *count)
>         if (!ops->get_count)
>                 return -ENOSYS;
> 
> -       return ops->get_count(dev, count);
> +       *count = ops->get_count(dev);
> +       return 0;
>  }
> 
>  unsigned long notrace timer_get_rate(struct udevice *dev)
> diff --git a/drivers/timer/tsc_timer.c b/drivers/timer/tsc_timer.c
> index 93c959ff44..abc0a1da05 100644
> --- a/drivers/timer/tsc_timer.c
> +++ b/drivers/timer/tsc_timer.c
> @@ -386,13 +386,11 @@ void __udelay(unsigned long usec)
>  #endif
>  }
> 
> -static int tsc_timer_get_count(struct udevice *dev, u64 *count)
> +static u64 tsc_timer_get_count(struct udevice *dev)
>  {
>         u64 now_tick = rdtsc();
> 
> -       *count = now_tick - gd->arch.tsc_base;
> -
> -       return 0;
> +       return now_tick - gd->arch.tsc_base;
>  }
> 
>  static void tsc_timer_ensure_setup(bool early)
> diff --git a/include/timer.h b/include/timer.h
> index aa9d870619..a044cb034e 100644
> --- a/include/timer.h
> +++ b/include/timer.h
> @@ -67,11 +67,14 @@ struct timer_ops {
>          *
>          * @dev: The timer device
>          *
> -        * @count: pointer that returns the current 64-bit timer count
> +        * This function may be called at any time after the driver is probed.
> +        * All necessary initialization must be completed by the time probe()
> +        * returns. The count returned by this functions should be monotonic.
> +        * This function must succeed.
>          *
> -        * Return: 0 if OK, -ve on error
> +        * Return: The current 64-bit timer count
>          */
> -       int (*get_count)(struct udevice *dev, u64 *count);
> +       u64 (*get_count)(struct udevice *dev);
>  };
> 
>  /**
> --
> 2.28.0
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 2/2] timer: Return count from timer_ops.get_count
  2020-10-07 18:37 ` [PATCH v2 2/2] timer: Return count from timer_ops.get_count Sean Anderson
  2020-10-08  8:28   ` Claudiu.Beznea at microchip.com
@ 2020-10-09 17:23   ` Simon Glass
  2020-10-23  0:27   ` Tom Rini
  2 siblings, 0 replies; 8+ messages in thread
From: Simon Glass @ 2020-10-09 17:23 UTC (permalink / raw)
  To: u-boot

Hi Sean,

On Wed, 7 Oct 2020 at 12:38, Sean Anderson <seanga2@gmail.com> wrote:
>
> No timer drivers return an error from get_count. Instead of possibly
> returning an error, just return the count directly.
>
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> ---
>
> Changes in v2:
> - mchp-pit64b was added since v1, so convert it
> - Document when get_count may be called, and what assumptions the timer
>   subsystem makes about drivers
>
>  arch/riscv/lib/andes_plmt.c       |  6 ++----
>  arch/riscv/lib/sifive_clint.c     |  6 ++----
>  drivers/timer/ag101p_timer.c      |  5 ++---
>  drivers/timer/altera_timer.c      |  6 ++----
>  drivers/timer/arc_timer.c         |  6 ++----
>  drivers/timer/ast_timer.c         |  6 ++----
>  drivers/timer/atcpit100_timer.c   |  5 ++---
>  drivers/timer/atmel_pit_timer.c   |  6 ++----
>  drivers/timer/cadence-ttc.c       |  6 ++----
>  drivers/timer/dw-apb-timer.c      |  6 ++----
>  drivers/timer/mchp-pit64b-timer.c |  6 ++----
>  drivers/timer/mpc83xx_timer.c     |  6 ++----
>  drivers/timer/mtk_timer.c         |  6 ++----
>  drivers/timer/nomadik-mtu-timer.c |  6 ++----
>  drivers/timer/omap-timer.c        |  6 ++----
>  drivers/timer/ostm_timer.c        |  6 ++----
>  drivers/timer/riscv_timer.c       | 21 +++++++++------------
>  drivers/timer/rockchip_timer.c    |  5 ++---
>  drivers/timer/sandbox_timer.c     |  6 ++----
>  drivers/timer/sti-timer.c         |  6 ++----
>  drivers/timer/stm32_timer.c       |  6 ++----
>  drivers/timer/timer-uclass.c      |  3 ++-
>  drivers/timer/tsc_timer.c         |  6 ++----
>  include/timer.h                   |  9 ++++++---
>  24 files changed, 59 insertions(+), 97 deletions(-)
>

[..]

> diff --git a/include/timer.h b/include/timer.h
> index aa9d870619..a044cb034e 100644
> --- a/include/timer.h
> +++ b/include/timer.h
> @@ -67,11 +67,14 @@ struct timer_ops {
>          *
>          * @dev: The timer device
>          *
> -        * @count: pointer that returns the current 64-bit timer count
> +        * This function may be called at any time after the driver is probed.
> +        * All necessary initialization must be completed by the time probe()
> +        * returns. The count returned by this functions should be monotonic.
> +        * This function must succeed.
>          *
> -        * Return: 0 if OK, -ve on error
> +        * Return: The current 64-bit timer count
>          */
> -       int (*get_count)(struct udevice *dev, u64 *count);
> +       u64 (*get_count)(struct udevice *dev);

We can require this the driver be probed before this is called. We
have the early timer for the pre-DM case.

Reviewed-by: Simon Glass <sjg@chromium.org>

Regards,
Simon

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 1/2] doc: Document timer API
  2020-10-07 18:37 ` [PATCH v2 1/2] doc: Document timer API Sean Anderson
@ 2020-10-12  3:34   ` Simon Glass
  2020-10-23  0:27   ` Tom Rini
  1 sibling, 0 replies; 8+ messages in thread
From: Simon Glass @ 2020-10-12  3:34 UTC (permalink / raw)
  To: u-boot

On Wed, 7 Oct 2020 at 12:37, Sean Anderson <seanga2@gmail.com> wrote:
>
> This adds kerneldocs for <timer.h>.
>
> I don't know who should maintain doc/api/timer.rst, since the timer
> subsystem seems to be maintained by SoC maintainers. MAINTAINERS is left
> un-updated for the moment.
>
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> ---
>
> Changes in v2:
> - New
>
>  doc/api/index.rst |  1 +
>  doc/api/timer.rst |  8 ++++++++
>  include/timer.h   | 46 ++++++++++++++++++++++++----------------------
>  3 files changed, 33 insertions(+), 22 deletions(-)
>  create mode 100644 doc/api/timer.rst

Reviewed-by: Simon Glass <sjg@chromium.org>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 1/2] doc: Document timer API
  2020-10-07 18:37 ` [PATCH v2 1/2] doc: Document timer API Sean Anderson
  2020-10-12  3:34   ` Simon Glass
@ 2020-10-23  0:27   ` Tom Rini
  1 sibling, 0 replies; 8+ messages in thread
From: Tom Rini @ 2020-10-23  0:27 UTC (permalink / raw)
  To: u-boot

On Wed, Oct 07, 2020 at 02:37:43PM -0400, Sean Anderson wrote:

> This adds kerneldocs for <timer.h>.
> 
> I don't know who should maintain doc/api/timer.rst, since the timer
> subsystem seems to be maintained by SoC maintainers. MAINTAINERS is left
> un-updated for the moment.
> 
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201022/dba11bc6/attachment.sig>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 2/2] timer: Return count from timer_ops.get_count
  2020-10-07 18:37 ` [PATCH v2 2/2] timer: Return count from timer_ops.get_count Sean Anderson
  2020-10-08  8:28   ` Claudiu.Beznea at microchip.com
  2020-10-09 17:23   ` Simon Glass
@ 2020-10-23  0:27   ` Tom Rini
  2 siblings, 0 replies; 8+ messages in thread
From: Tom Rini @ 2020-10-23  0:27 UTC (permalink / raw)
  To: u-boot

On Wed, Oct 07, 2020 at 02:37:44PM -0400, Sean Anderson wrote:

> No timer drivers return an error from get_count. Instead of possibly
> returning an error, just return the count directly.
> 
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> Reviewed-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201022/c4cffd48/attachment-0001.sig>

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-10-23  0:27 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-07 18:37 [PATCH v2 0/2] timer: Return a u64 directly Sean Anderson
2020-10-07 18:37 ` [PATCH v2 1/2] doc: Document timer API Sean Anderson
2020-10-12  3:34   ` Simon Glass
2020-10-23  0:27   ` Tom Rini
2020-10-07 18:37 ` [PATCH v2 2/2] timer: Return count from timer_ops.get_count Sean Anderson
2020-10-08  8:28   ` Claudiu.Beznea at microchip.com
2020-10-09 17:23   ` Simon Glass
2020-10-23  0:27   ` Tom Rini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.