thermal: tegra: Nuke clk_{readl,writel} helpers
diff mbox

Message ID 20180331001004.16174-1-ezequiel@collabora.com
State Accepted
Delegated to: Eduardo Valentin
Headers show

Commit Message

Ezequiel Garcia March 31, 2018, 12:10 a.m. UTC
Naming driver-specific register accessors with generic
names, such as clk_writel and clk_readl, is bad.

Moreover, clk_writel and clk_readl are part of the
common clock framework api, so readers and code
grep'ers get confused by this collision.

The helpers are used once, so just remove them.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 drivers/thermal/tegra/soctherm.c | 29 ++---------------------------
 1 file changed, 2 insertions(+), 27 deletions(-)

Comments

Ezequiel Garcia April 13, 2018, 3:54 p.m. UTC | #1
On Fri, 2018-03-30 at 21:10 -0300, Ezequiel Garcia wrote:
> Naming driver-specific register accessors with generic
> names, such as clk_writel and clk_readl, is bad.
> 
> Moreover, clk_writel and clk_readl are part of the
> common clock framework api, so readers and code
> grep'ers get confused by this collision.
> 
> The helpers are used once, so just remove them.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>

Hey folks,

Any feedback?

Thanks,
Eze
Daniel Lezcano April 13, 2018, 3:58 p.m. UTC | #2
On 31/03/2018 02:10, Ezequiel Garcia wrote:
> Naming driver-specific register accessors with generic
> names, such as clk_writel and clk_readl, is bad.
> 
> Moreover, clk_writel and clk_readl are part of the
> common clock framework api, so readers and code
> grep'ers get confused by this collision.
> 
> The helpers are used once, so just remove them.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---

Makes totally sense.

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Ezequiel Garcia April 30, 2018, 5:15 p.m. UTC | #3
On Fri, 2018-04-13 at 17:58 +0200, Daniel Lezcano wrote:
> On 31/03/2018 02:10, Ezequiel Garcia wrote:
> > Naming driver-specific register accessors with generic
> > names, such as clk_writel and clk_readl, is bad.
> > 
> > Moreover, clk_writel and clk_readl are part of the
> > common clock framework api, so readers and code
> > grep'ers get confused by this collision.
> > 
> > The helpers are used once, so just remove them.
> > 
> > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> > ---
> 
> Makes totally sense.
> 
> Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> 

A month has passed since Daniel's Acked-by,
any plans to merge this little patch?

Thanks,
Eze

Patch
diff mbox

diff --git a/drivers/thermal/tegra/soctherm.c b/drivers/thermal/tegra/soctherm.c
index 455b58ce2652..1f87bbe7618b 100644
--- a/drivers/thermal/tegra/soctherm.c
+++ b/drivers/thermal/tegra/soctherm.c
@@ -240,31 +240,6 @@  struct tegra_soctherm {
 	struct dentry *debugfs_dir;
 };
 
-/**
- * clk_writel() - writes a value to a CAR register
- * @ts: pointer to a struct tegra_soctherm
- * @v: the value to write
- * @reg: the register offset
- *
- * Writes @v to @reg.  No return value.
- */
-static inline void clk_writel(struct tegra_soctherm *ts, u32 value, u32 reg)
-{
-	writel(value, (ts->clk_regs + reg));
-}
-
-/**
- * clk_readl() - reads specified register from CAR IP block
- * @ts: pointer to a struct tegra_soctherm
- * @reg: register address to be read
- *
- * Return: the value of the register
- */
-static inline u32 clk_readl(struct tegra_soctherm *ts, u32 reg)
-{
-	return readl(ts->clk_regs + reg);
-}
-
 /**
  * ccroc_writel() - writes a value to a CCROC register
  * @ts: pointer to a struct tegra_soctherm
@@ -1207,9 +1182,9 @@  static void tegra_soctherm_throttle(struct device *dev)
 	} else {
 		writel(v, ts->regs + THROT_GLOBAL_CFG);
 
-		v = clk_readl(ts, CAR_SUPER_CCLKG_DIVIDER);
+		v = readl(ts->clk_regs + CAR_SUPER_CCLKG_DIVIDER);
 		v = REG_SET_MASK(v, CDIVG_USE_THERM_CONTROLS_MASK, 1);
-		clk_writel(ts, v, CAR_SUPER_CCLKG_DIVIDER);
+		writel(v, ts->clk_regs + CAR_SUPER_CCLKG_DIVIDER);
 	}
 
 	/* initialize stats collection */