* [PATCH 2/2] clk: rockchip: remove ROCKCHIP_SOFTRST_HIWORD_MASK for rockchip platform
@ 2016-06-03 0:54 Shawn Lin
2016-06-03 1:25 ` Xing Zheng
0 siblings, 1 reply; 5+ messages in thread
From: Shawn Lin @ 2016-06-03 0:54 UTC (permalink / raw)
To: Heiko Stuebner
Cc: linux-clk, Stephen Boyd, Xing Zheng, linux-rockchip,
linux-kernel, Shawn Lin
I check all the Socs including RK2928/3000/3066/3028X/316X/312X/
3190/3188/3228/3368/3399/3036, and find all of them use high 16-bit
as write mask. Obviously we don't need ROCKCHIP_SOFTRST_HIWORD_MASK
any more(actually I don't know why we need it before). This patch
removes it to simplify the code and save a little cpu cycle when calling
assert or deassert callback.
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---
drivers/clk/rockchip/clk-rk3036.c | 3 +--
drivers/clk/rockchip/clk-rk3188.c | 3 +--
drivers/clk/rockchip/clk-rk3228.c | 3 +--
drivers/clk/rockchip/clk-rk3288.c | 3 +--
drivers/clk/rockchip/clk-rk3368.c | 3 +--
drivers/clk/rockchip/clk-rk3399.c | 6 ++----
drivers/clk/rockchip/clk.h | 6 ++----
drivers/clk/rockchip/softrst.c | 40 +++++----------------------------------
8 files changed, 14 insertions(+), 53 deletions(-)
diff --git a/drivers/clk/rockchip/clk-rk3036.c b/drivers/clk/rockchip/clk-rk3036.c
index 924f560..a13dfc2 100644
--- a/drivers/clk/rockchip/clk-rk3036.c
+++ b/drivers/clk/rockchip/clk-rk3036.c
@@ -475,8 +475,7 @@ static void __init rk3036_clk_init(struct device_node *np)
&rk3036_cpuclk_data, rk3036_cpuclk_rates,
ARRAY_SIZE(rk3036_cpuclk_rates));
- rockchip_register_softrst(np, 9, reg_base + RK2928_SOFTRST_CON(0),
- ROCKCHIP_SOFTRST_HIWORD_MASK);
+ rockchip_register_softrst(np, 9, reg_base + RK2928_SOFTRST_CON(0));
rockchip_register_restart_notifier(ctx, RK2928_GLB_SRST_FST, NULL);
diff --git a/drivers/clk/rockchip/clk-rk3188.c b/drivers/clk/rockchip/clk-rk3188.c
index d0e722a..4acd787 100644
--- a/drivers/clk/rockchip/clk-rk3188.c
+++ b/drivers/clk/rockchip/clk-rk3188.c
@@ -780,8 +780,7 @@ static struct rockchip_clk_provider *__init rk3188_common_clk_init(struct device
rockchip_clk_register_branches(ctx, common_clk_branches,
ARRAY_SIZE(common_clk_branches));
- rockchip_register_softrst(np, 9, reg_base + RK2928_SOFTRST_CON(0),
- ROCKCHIP_SOFTRST_HIWORD_MASK);
+ rockchip_register_softrst(np, 9, reg_base + RK2928_SOFTRST_CON(0));
rockchip_register_restart_notifier(ctx, RK2928_GLB_SRST_FST, NULL);
diff --git a/drivers/clk/rockchip/clk-rk3228.c b/drivers/clk/rockchip/clk-rk3228.c
index 016bdb0..8412753 100644
--- a/drivers/clk/rockchip/clk-rk3228.c
+++ b/drivers/clk/rockchip/clk-rk3228.c
@@ -657,8 +657,7 @@ static void __init rk3228_clk_init(struct device_node *np)
&rk3228_cpuclk_data, rk3228_cpuclk_rates,
ARRAY_SIZE(rk3228_cpuclk_rates));
- rockchip_register_softrst(np, 9, reg_base + RK2928_SOFTRST_CON(0),
- ROCKCHIP_SOFTRST_HIWORD_MASK);
+ rockchip_register_softrst(np, 9, reg_base + RK2928_SOFTRST_CON(0));
rockchip_register_restart_notifier(ctx, RK3228_GLB_SRST_FST, NULL);
diff --git a/drivers/clk/rockchip/clk-rk3288.c b/drivers/clk/rockchip/clk-rk3288.c
index 39af05a..a779a82 100644
--- a/drivers/clk/rockchip/clk-rk3288.c
+++ b/drivers/clk/rockchip/clk-rk3288.c
@@ -919,8 +919,7 @@ static void __init rk3288_clk_init(struct device_node *np)
ARRAY_SIZE(rk3288_cpuclk_rates));
rockchip_register_softrst(np, 12,
- rk3288_cru_base + RK3288_SOFTRST_CON(0),
- ROCKCHIP_SOFTRST_HIWORD_MASK);
+ rk3288_cru_base + RK3288_SOFTRST_CON(0));
rockchip_register_restart_notifier(ctx, RK3288_GLB_SRST_FST,
rk3288_clk_shutdown);
diff --git a/drivers/clk/rockchip/clk-rk3368.c b/drivers/clk/rockchip/clk-rk3368.c
index 6cb474c..e0f8c73 100644
--- a/drivers/clk/rockchip/clk-rk3368.c
+++ b/drivers/clk/rockchip/clk-rk3368.c
@@ -905,8 +905,7 @@ static void __init rk3368_clk_init(struct device_node *np)
&rk3368_cpuclkl_data, rk3368_cpuclkl_rates,
ARRAY_SIZE(rk3368_cpuclkl_rates));
- rockchip_register_softrst(np, 15, reg_base + RK3368_SOFTRST_CON(0),
- ROCKCHIP_SOFTRST_HIWORD_MASK);
+ rockchip_register_softrst(np, 15, reg_base + RK3368_SOFTRST_CON(0));
rockchip_register_restart_notifier(ctx, RK3368_GLB_SRST_FST, NULL);
diff --git a/drivers/clk/rockchip/clk-rk3399.c b/drivers/clk/rockchip/clk-rk3399.c
index 37c74cb..4601a5c 100644
--- a/drivers/clk/rockchip/clk-rk3399.c
+++ b/drivers/clk/rockchip/clk-rk3399.c
@@ -1540,8 +1540,7 @@ static void __init rk3399_clk_init(struct device_node *np)
&rk3399_cpuclkb_data, rk3399_cpuclkb_rates,
ARRAY_SIZE(rk3399_cpuclkb_rates));
- rockchip_register_softrst(np, 21, reg_base + RK3399_SOFTRST_CON(0),
- ROCKCHIP_SOFTRST_HIWORD_MASK);
+ rockchip_register_softrst(np, 21, reg_base + RK3399_SOFTRST_CON(0));
rockchip_register_restart_notifier(ctx, RK3399_GLB_SRST_FST, NULL);
@@ -1576,8 +1575,7 @@ static void __init rk3399_pmu_clk_init(struct device_node *np)
rockchip_clk_protect_critical(rk3399_pmucru_critical_clocks,
ARRAY_SIZE(rk3399_pmucru_critical_clocks));
- rockchip_register_softrst(np, 2, reg_base + RK3399_PMU_SOFTRST_CON(0),
- ROCKCHIP_SOFTRST_HIWORD_MASK);
+ rockchip_register_softrst(np, 2, reg_base + RK3399_PMU_SOFTRST_CON(0));
rockchip_clk_of_add_provider(np, ctx);
}
diff --git a/drivers/clk/rockchip/clk.h b/drivers/clk/rockchip/clk.h
index 2194ffa..51e797c 100644
--- a/drivers/clk/rockchip/clk.h
+++ b/drivers/clk/rockchip/clk.h
@@ -618,16 +618,14 @@ void rockchip_clk_protect_critical(const char *const clocks[], int nclocks);
void rockchip_register_restart_notifier(struct rockchip_clk_provider *ctx,
unsigned int reg, void (*cb)(void));
-#define ROCKCHIP_SOFTRST_HIWORD_MASK BIT(0)
-
#ifdef CONFIG_RESET_CONTROLLER
void rockchip_register_softrst(struct device_node *np,
unsigned int num_regs,
- void __iomem *base, u8 flags);
+ void __iomem *base);
#else
static inline void rockchip_register_softrst(struct device_node *np,
unsigned int num_regs,
- void __iomem *base, u8 flags)
+ void __iomem *base)
{
}
#endif
diff --git a/drivers/clk/rockchip/softrst.c b/drivers/clk/rockchip/softrst.c
index 21218987..d6b865f 100644
--- a/drivers/clk/rockchip/softrst.c
+++ b/drivers/clk/rockchip/softrst.c
@@ -24,8 +24,6 @@ struct rockchip_softrst {
void __iomem *reg_base;
int num_regs;
int num_per_reg;
- u8 flags;
- spinlock_t lock;
};
static int rockchip_softrst_assert(struct reset_controller_dev *rcdev,
@@ -37,20 +35,8 @@ static int rockchip_softrst_assert(struct reset_controller_dev *rcdev,
int bank = id / softrst->num_per_reg;
int offset = id % softrst->num_per_reg;
- if (softrst->flags & ROCKCHIP_SOFTRST_HIWORD_MASK) {
- writel(BIT(offset) | (BIT(offset) << 16),
- softrst->reg_base + (bank * 4));
- } else {
- unsigned long flags;
- u32 reg;
-
- spin_lock_irqsave(&softrst->lock, flags);
-
- reg = readl(softrst->reg_base + (bank * 4));
- writel(reg | BIT(offset), softrst->reg_base + (bank * 4));
-
- spin_unlock_irqrestore(&softrst->lock, flags);
- }
+ writel(BIT(offset) | (BIT(offset) << 16),
+ softrst->reg_base + (bank * 4));
return 0;
}
@@ -64,19 +50,7 @@ static int rockchip_softrst_deassert(struct reset_controller_dev *rcdev,
int bank = id / softrst->num_per_reg;
int offset = id % softrst->num_per_reg;
- if (softrst->flags & ROCKCHIP_SOFTRST_HIWORD_MASK) {
- writel((BIT(offset) << 16), softrst->reg_base + (bank * 4));
- } else {
- unsigned long flags;
- u32 reg;
-
- spin_lock_irqsave(&softrst->lock, flags);
-
- reg = readl(softrst->reg_base + (bank * 4));
- writel(reg & ~BIT(offset), softrst->reg_base + (bank * 4));
-
- spin_unlock_irqrestore(&softrst->lock, flags);
- }
+ writel((BIT(offset) << 16), softrst->reg_base + (bank * 4));
return 0;
}
@@ -88,7 +62,7 @@ static const struct reset_control_ops rockchip_softrst_ops = {
void __init rockchip_register_softrst(struct device_node *np,
unsigned int num_regs,
- void __iomem *base, u8 flags)
+ void __iomem *base)
{
struct rockchip_softrst *softrst;
int ret;
@@ -97,13 +71,9 @@ void __init rockchip_register_softrst(struct device_node *np,
if (!softrst)
return;
- spin_lock_init(&softrst->lock);
-
softrst->reg_base = base;
- softrst->flags = flags;
softrst->num_regs = num_regs;
- softrst->num_per_reg = (flags & ROCKCHIP_SOFTRST_HIWORD_MASK) ? 16
- : 32;
+ softrst->num_per_reg = 16;
softrst->rcdev.owner = THIS_MODULE;
softrst->rcdev.nr_resets = num_regs * softrst->num_per_reg;
--
2.3.7
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] clk: rockchip: remove ROCKCHIP_SOFTRST_HIWORD_MASK for rockchip platform
2016-06-03 0:54 [PATCH 2/2] clk: rockchip: remove ROCKCHIP_SOFTRST_HIWORD_MASK for rockchip platform Shawn Lin
@ 2016-06-03 1:25 ` Xing Zheng
2016-06-03 3:35 ` Shawn Lin
0 siblings, 1 reply; 5+ messages in thread
From: Xing Zheng @ 2016-06-03 1:25 UTC (permalink / raw)
To: Shawn Lin
Cc: Heiko Stuebner, linux-clk, Stephen Boyd, linux-rockchip, linux-kernel
Hi Shawn,
On 2016年06月03日 08:54, Shawn Lin wrote:
> I check all the Socs including RK2928/3000/3066/3028X/316X/312X/
> 3190/3188/3228/3368/3399/3036, and find all of them use high 16-bit
> as write mask. Obviously we don't need ROCKCHIP_SOFTRST_HIWORD_MASK
> any more(actually I don't know why we need it before). This patch
> removes it to simplify the code and save a little cpu cycle when calling
> assert or deassert callback.
In my opinion, this flag can be used for compatibility, we can not
ensure that our SoCs will not use the 32bit SOFTRST_CONs in future.
Thanks.
>
> Signed-off-by: Shawn Lin<shawn.lin@rock-chips.com>
> ---
>
> drivers/clk/rockchip/clk-rk3036.c | 3 +--
> drivers/clk/rockchip/clk-rk3188.c | 3 +--
> drivers/clk/rockchip/clk-rk3228.c | 3 +--
> drivers/clk/rockchip/clk-rk3288.c | 3 +--
> drivers/clk/rockchip/clk-rk3368.c | 3 +--
> drivers/clk/rockchip/clk-rk3399.c | 6 ++----
> drivers/clk/rockchip/clk.h | 6 ++----
> drivers/clk/rockchip/softrst.c | 40 +++++----------------------------------
> 8 files changed, 14 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/clk/rockchip/clk-rk3036.c b/drivers/clk/rockchip/clk-rk3036.c
> index 924f560..a13dfc2 100644
> --- a/drivers/clk/rockchip/clk-rk3036.c
> +++ b/drivers/clk/rockchip/clk-rk3036.c
> @@ -475,8 +475,7 @@ static void __init rk3036_clk_init(struct device_node *np)
> &rk3036_cpuclk_data, rk3036_cpuclk_rates,
> ARRAY_SIZE(rk3036_cpuclk_rates));
>
> - rockchip_register_softrst(np, 9, reg_base + RK2928_SOFTRST_CON(0),
> - ROCKCHIP_SOFTRST_HIWORD_MASK);
> + rockchip_register_softrst(np, 9, reg_base + RK2928_SOFTRST_CON(0));
>
> rockchip_register_restart_notifier(ctx, RK2928_GLB_SRST_FST, NULL);
>
> diff --git a/drivers/clk/rockchip/clk-rk3188.c b/drivers/clk/rockchip/clk-rk3188.c
> index d0e722a..4acd787 100644
> --- a/drivers/clk/rockchip/clk-rk3188.c
> +++ b/drivers/clk/rockchip/clk-rk3188.c
> @@ -780,8 +780,7 @@ static struct rockchip_clk_provider *__init rk3188_common_clk_init(struct device
> rockchip_clk_register_branches(ctx, common_clk_branches,
> ARRAY_SIZE(common_clk_branches));
>
> - rockchip_register_softrst(np, 9, reg_base + RK2928_SOFTRST_CON(0),
> - ROCKCHIP_SOFTRST_HIWORD_MASK);
> + rockchip_register_softrst(np, 9, reg_base + RK2928_SOFTRST_CON(0));
>
> rockchip_register_restart_notifier(ctx, RK2928_GLB_SRST_FST, NULL);
>
> diff --git a/drivers/clk/rockchip/clk-rk3228.c b/drivers/clk/rockchip/clk-rk3228.c
> index 016bdb0..8412753 100644
> --- a/drivers/clk/rockchip/clk-rk3228.c
> +++ b/drivers/clk/rockchip/clk-rk3228.c
> @@ -657,8 +657,7 @@ static void __init rk3228_clk_init(struct device_node *np)
> &rk3228_cpuclk_data, rk3228_cpuclk_rates,
> ARRAY_SIZE(rk3228_cpuclk_rates));
>
> - rockchip_register_softrst(np, 9, reg_base + RK2928_SOFTRST_CON(0),
> - ROCKCHIP_SOFTRST_HIWORD_MASK);
> + rockchip_register_softrst(np, 9, reg_base + RK2928_SOFTRST_CON(0));
>
> rockchip_register_restart_notifier(ctx, RK3228_GLB_SRST_FST, NULL);
>
> diff --git a/drivers/clk/rockchip/clk-rk3288.c b/drivers/clk/rockchip/clk-rk3288.c
> index 39af05a..a779a82 100644
> --- a/drivers/clk/rockchip/clk-rk3288.c
> +++ b/drivers/clk/rockchip/clk-rk3288.c
> @@ -919,8 +919,7 @@ static void __init rk3288_clk_init(struct device_node *np)
> ARRAY_SIZE(rk3288_cpuclk_rates));
>
> rockchip_register_softrst(np, 12,
> - rk3288_cru_base + RK3288_SOFTRST_CON(0),
> - ROCKCHIP_SOFTRST_HIWORD_MASK);
> + rk3288_cru_base + RK3288_SOFTRST_CON(0));
>
> rockchip_register_restart_notifier(ctx, RK3288_GLB_SRST_FST,
> rk3288_clk_shutdown);
> diff --git a/drivers/clk/rockchip/clk-rk3368.c b/drivers/clk/rockchip/clk-rk3368.c
> index 6cb474c..e0f8c73 100644
> --- a/drivers/clk/rockchip/clk-rk3368.c
> +++ b/drivers/clk/rockchip/clk-rk3368.c
> @@ -905,8 +905,7 @@ static void __init rk3368_clk_init(struct device_node *np)
> &rk3368_cpuclkl_data, rk3368_cpuclkl_rates,
> ARRAY_SIZE(rk3368_cpuclkl_rates));
>
> - rockchip_register_softrst(np, 15, reg_base + RK3368_SOFTRST_CON(0),
> - ROCKCHIP_SOFTRST_HIWORD_MASK);
> + rockchip_register_softrst(np, 15, reg_base + RK3368_SOFTRST_CON(0));
>
> rockchip_register_restart_notifier(ctx, RK3368_GLB_SRST_FST, NULL);
>
> diff --git a/drivers/clk/rockchip/clk-rk3399.c b/drivers/clk/rockchip/clk-rk3399.c
> index 37c74cb..4601a5c 100644
> --- a/drivers/clk/rockchip/clk-rk3399.c
> +++ b/drivers/clk/rockchip/clk-rk3399.c
> @@ -1540,8 +1540,7 @@ static void __init rk3399_clk_init(struct device_node *np)
> &rk3399_cpuclkb_data, rk3399_cpuclkb_rates,
> ARRAY_SIZE(rk3399_cpuclkb_rates));
>
> - rockchip_register_softrst(np, 21, reg_base + RK3399_SOFTRST_CON(0),
> - ROCKCHIP_SOFTRST_HIWORD_MASK);
> + rockchip_register_softrst(np, 21, reg_base + RK3399_SOFTRST_CON(0));
>
> rockchip_register_restart_notifier(ctx, RK3399_GLB_SRST_FST, NULL);
>
> @@ -1576,8 +1575,7 @@ static void __init rk3399_pmu_clk_init(struct device_node *np)
> rockchip_clk_protect_critical(rk3399_pmucru_critical_clocks,
> ARRAY_SIZE(rk3399_pmucru_critical_clocks));
>
> - rockchip_register_softrst(np, 2, reg_base + RK3399_PMU_SOFTRST_CON(0),
> - ROCKCHIP_SOFTRST_HIWORD_MASK);
> + rockchip_register_softrst(np, 2, reg_base + RK3399_PMU_SOFTRST_CON(0));
>
> rockchip_clk_of_add_provider(np, ctx);
> }
> diff --git a/drivers/clk/rockchip/clk.h b/drivers/clk/rockchip/clk.h
> index 2194ffa..51e797c 100644
> --- a/drivers/clk/rockchip/clk.h
> +++ b/drivers/clk/rockchip/clk.h
> @@ -618,16 +618,14 @@ void rockchip_clk_protect_critical(const char *const clocks[], int nclocks);
> void rockchip_register_restart_notifier(struct rockchip_clk_provider *ctx,
> unsigned int reg, void (*cb)(void));
>
> -#define ROCKCHIP_SOFTRST_HIWORD_MASK BIT(0)
> -
> #ifdef CONFIG_RESET_CONTROLLER
> void rockchip_register_softrst(struct device_node *np,
> unsigned int num_regs,
> - void __iomem *base, u8 flags);
> + void __iomem *base);
> #else
> static inline void rockchip_register_softrst(struct device_node *np,
> unsigned int num_regs,
> - void __iomem *base, u8 flags)
> + void __iomem *base)
> {
> }
> #endif
> diff --git a/drivers/clk/rockchip/softrst.c b/drivers/clk/rockchip/softrst.c
> index 21218987..d6b865f 100644
> --- a/drivers/clk/rockchip/softrst.c
> +++ b/drivers/clk/rockchip/softrst.c
> @@ -24,8 +24,6 @@ struct rockchip_softrst {
> void __iomem *reg_base;
> int num_regs;
> int num_per_reg;
> - u8 flags;
> - spinlock_t lock;
> };
>
> static int rockchip_softrst_assert(struct reset_controller_dev *rcdev,
> @@ -37,20 +35,8 @@ static int rockchip_softrst_assert(struct reset_controller_dev *rcdev,
> int bank = id / softrst->num_per_reg;
> int offset = id % softrst->num_per_reg;
>
> - if (softrst->flags& ROCKCHIP_SOFTRST_HIWORD_MASK) {
> - writel(BIT(offset) | (BIT(offset)<< 16),
> - softrst->reg_base + (bank * 4));
> - } else {
> - unsigned long flags;
> - u32 reg;
> -
> - spin_lock_irqsave(&softrst->lock, flags);
> -
> - reg = readl(softrst->reg_base + (bank * 4));
> - writel(reg | BIT(offset), softrst->reg_base + (bank * 4));
> -
> - spin_unlock_irqrestore(&softrst->lock, flags);
> - }
> + writel(BIT(offset) | (BIT(offset)<< 16),
> + softrst->reg_base + (bank * 4));
>
> return 0;
> }
> @@ -64,19 +50,7 @@ static int rockchip_softrst_deassert(struct reset_controller_dev *rcdev,
> int bank = id / softrst->num_per_reg;
> int offset = id % softrst->num_per_reg;
>
> - if (softrst->flags& ROCKCHIP_SOFTRST_HIWORD_MASK) {
> - writel((BIT(offset)<< 16), softrst->reg_base + (bank * 4));
> - } else {
> - unsigned long flags;
> - u32 reg;
> -
> - spin_lock_irqsave(&softrst->lock, flags);
> -
> - reg = readl(softrst->reg_base + (bank * 4));
> - writel(reg& ~BIT(offset), softrst->reg_base + (bank * 4));
> -
> - spin_unlock_irqrestore(&softrst->lock, flags);
> - }
> + writel((BIT(offset)<< 16), softrst->reg_base + (bank * 4));
>
> return 0;
> }
> @@ -88,7 +62,7 @@ static const struct reset_control_ops rockchip_softrst_ops = {
>
> void __init rockchip_register_softrst(struct device_node *np,
> unsigned int num_regs,
> - void __iomem *base, u8 flags)
> + void __iomem *base)
> {
> struct rockchip_softrst *softrst;
> int ret;
> @@ -97,13 +71,9 @@ void __init rockchip_register_softrst(struct device_node *np,
> if (!softrst)
> return;
>
> - spin_lock_init(&softrst->lock);
> -
> softrst->reg_base = base;
> - softrst->flags = flags;
> softrst->num_regs = num_regs;
> - softrst->num_per_reg = (flags& ROCKCHIP_SOFTRST_HIWORD_MASK) ? 16
> - : 32;
> + softrst->num_per_reg = 16;
>
> softrst->rcdev.owner = THIS_MODULE;
> softrst->rcdev.nr_resets = num_regs * softrst->num_per_reg;
--
- Xing Zheng
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] clk: rockchip: remove ROCKCHIP_SOFTRST_HIWORD_MASK for rockchip platform
2016-06-03 1:25 ` Xing Zheng
@ 2016-06-03 3:35 ` Shawn Lin
2016-06-03 7:29 ` Heiko Stübner
0 siblings, 1 reply; 5+ messages in thread
From: Shawn Lin @ 2016-06-03 3:35 UTC (permalink / raw)
To: Xing Zheng, Shawn Lin
Cc: linux-rockchip, Stephen Boyd, Heiko Stuebner, linux-kernel, linux-clk
在 2016/6/3 9:25, Xing Zheng 写道:
> Hi Shawn,
>
> On 2016年06月03日 08:54, Shawn Lin wrote:
>> I check all the Socs including RK2928/3000/3066/3028X/316X/312X/
>> 3190/3188/3228/3368/3399/3036, and find all of them use high 16-bit
>> as write mask. Obviously we don't need ROCKCHIP_SOFTRST_HIWORD_MASK
>> any more(actually I don't know why we need it before). This patch
>> removes it to simplify the code and save a little cpu cycle when calling
>> assert or deassert callback.
> In my opinion, this flag can be used for compatibility, we can not
> ensure that our SoCs will not use the 32bit SOFTRST_CONs in future.
>
> Thanks.
Thanks for sharing your thought.
I'm not 100% sure, but I'm 99% sure about we won't let
it happened. You have to consider the backward compatibility
rather than the future ones. If you got a chip with 10 bit, or
8bit for SOFTRST_CONX, so how do you wanna deal with it?
Should we now add ROCKCHIP_SOFTRST_X_BIT_MASK? :)
>>
>> Signed-off-by: Shawn Lin<shawn.lin@rock-chips.com>
>> ---
>>
>> drivers/clk/rockchip/clk-rk3036.c | 3 +--
>> drivers/clk/rockchip/clk-rk3188.c | 3 +--
>> drivers/clk/rockchip/clk-rk3228.c | 3 +--
>> drivers/clk/rockchip/clk-rk3288.c | 3 +--
>> drivers/clk/rockchip/clk-rk3368.c | 3 +--
>> drivers/clk/rockchip/clk-rk3399.c | 6 ++----
>> drivers/clk/rockchip/clk.h | 6 ++----
>> drivers/clk/rockchip/softrst.c | 40
>> +++++----------------------------------
>> 8 files changed, 14 insertions(+), 53 deletions(-)
>>
>> diff --git a/drivers/clk/rockchip/clk-rk3036.c
>> b/drivers/clk/rockchip/clk-rk3036.c
>> index 924f560..a13dfc2 100644
>> --- a/drivers/clk/rockchip/clk-rk3036.c
>> +++ b/drivers/clk/rockchip/clk-rk3036.c
>> @@ -475,8 +475,7 @@ static void __init rk3036_clk_init(struct
>> device_node *np)
>> &rk3036_cpuclk_data, rk3036_cpuclk_rates,
>> ARRAY_SIZE(rk3036_cpuclk_rates));
>>
>> - rockchip_register_softrst(np, 9, reg_base + RK2928_SOFTRST_CON(0),
>> - ROCKCHIP_SOFTRST_HIWORD_MASK);
>> + rockchip_register_softrst(np, 9, reg_base + RK2928_SOFTRST_CON(0));
>>
>> rockchip_register_restart_notifier(ctx, RK2928_GLB_SRST_FST, NULL);
>>
>> diff --git a/drivers/clk/rockchip/clk-rk3188.c
>> b/drivers/clk/rockchip/clk-rk3188.c
>> index d0e722a..4acd787 100644
>> --- a/drivers/clk/rockchip/clk-rk3188.c
>> +++ b/drivers/clk/rockchip/clk-rk3188.c
>> @@ -780,8 +780,7 @@ static struct rockchip_clk_provider *__init
>> rk3188_common_clk_init(struct device
>> rockchip_clk_register_branches(ctx, common_clk_branches,
>> ARRAY_SIZE(common_clk_branches));
>>
>> - rockchip_register_softrst(np, 9, reg_base + RK2928_SOFTRST_CON(0),
>> - ROCKCHIP_SOFTRST_HIWORD_MASK);
>> + rockchip_register_softrst(np, 9, reg_base + RK2928_SOFTRST_CON(0));
>>
>> rockchip_register_restart_notifier(ctx, RK2928_GLB_SRST_FST, NULL);
>>
>> diff --git a/drivers/clk/rockchip/clk-rk3228.c
>> b/drivers/clk/rockchip/clk-rk3228.c
>> index 016bdb0..8412753 100644
>> --- a/drivers/clk/rockchip/clk-rk3228.c
>> +++ b/drivers/clk/rockchip/clk-rk3228.c
>> @@ -657,8 +657,7 @@ static void __init rk3228_clk_init(struct
>> device_node *np)
>> &rk3228_cpuclk_data, rk3228_cpuclk_rates,
>> ARRAY_SIZE(rk3228_cpuclk_rates));
>>
>> - rockchip_register_softrst(np, 9, reg_base + RK2928_SOFTRST_CON(0),
>> - ROCKCHIP_SOFTRST_HIWORD_MASK);
>> + rockchip_register_softrst(np, 9, reg_base + RK2928_SOFTRST_CON(0));
>>
>> rockchip_register_restart_notifier(ctx, RK3228_GLB_SRST_FST, NULL);
>>
>> diff --git a/drivers/clk/rockchip/clk-rk3288.c
>> b/drivers/clk/rockchip/clk-rk3288.c
>> index 39af05a..a779a82 100644
>> --- a/drivers/clk/rockchip/clk-rk3288.c
>> +++ b/drivers/clk/rockchip/clk-rk3288.c
>> @@ -919,8 +919,7 @@ static void __init rk3288_clk_init(struct
>> device_node *np)
>> ARRAY_SIZE(rk3288_cpuclk_rates));
>>
>> rockchip_register_softrst(np, 12,
>> - rk3288_cru_base + RK3288_SOFTRST_CON(0),
>> - ROCKCHIP_SOFTRST_HIWORD_MASK);
>> + rk3288_cru_base + RK3288_SOFTRST_CON(0));
>>
>> rockchip_register_restart_notifier(ctx, RK3288_GLB_SRST_FST,
>> rk3288_clk_shutdown);
>> diff --git a/drivers/clk/rockchip/clk-rk3368.c
>> b/drivers/clk/rockchip/clk-rk3368.c
>> index 6cb474c..e0f8c73 100644
>> --- a/drivers/clk/rockchip/clk-rk3368.c
>> +++ b/drivers/clk/rockchip/clk-rk3368.c
>> @@ -905,8 +905,7 @@ static void __init rk3368_clk_init(struct
>> device_node *np)
>> &rk3368_cpuclkl_data, rk3368_cpuclkl_rates,
>> ARRAY_SIZE(rk3368_cpuclkl_rates));
>>
>> - rockchip_register_softrst(np, 15, reg_base + RK3368_SOFTRST_CON(0),
>> - ROCKCHIP_SOFTRST_HIWORD_MASK);
>> + rockchip_register_softrst(np, 15, reg_base + RK3368_SOFTRST_CON(0));
>>
>> rockchip_register_restart_notifier(ctx, RK3368_GLB_SRST_FST, NULL);
>>
>> diff --git a/drivers/clk/rockchip/clk-rk3399.c
>> b/drivers/clk/rockchip/clk-rk3399.c
>> index 37c74cb..4601a5c 100644
>> --- a/drivers/clk/rockchip/clk-rk3399.c
>> +++ b/drivers/clk/rockchip/clk-rk3399.c
>> @@ -1540,8 +1540,7 @@ static void __init rk3399_clk_init(struct
>> device_node *np)
>> &rk3399_cpuclkb_data, rk3399_cpuclkb_rates,
>> ARRAY_SIZE(rk3399_cpuclkb_rates));
>>
>> - rockchip_register_softrst(np, 21, reg_base + RK3399_SOFTRST_CON(0),
>> - ROCKCHIP_SOFTRST_HIWORD_MASK);
>> + rockchip_register_softrst(np, 21, reg_base + RK3399_SOFTRST_CON(0));
>>
>> rockchip_register_restart_notifier(ctx, RK3399_GLB_SRST_FST, NULL);
>>
>> @@ -1576,8 +1575,7 @@ static void __init rk3399_pmu_clk_init(struct
>> device_node *np)
>> rockchip_clk_protect_critical(rk3399_pmucru_critical_clocks,
>> ARRAY_SIZE(rk3399_pmucru_critical_clocks));
>>
>> - rockchip_register_softrst(np, 2, reg_base +
>> RK3399_PMU_SOFTRST_CON(0),
>> - ROCKCHIP_SOFTRST_HIWORD_MASK);
>> + rockchip_register_softrst(np, 2, reg_base +
>> RK3399_PMU_SOFTRST_CON(0));
>>
>> rockchip_clk_of_add_provider(np, ctx);
>> }
>> diff --git a/drivers/clk/rockchip/clk.h b/drivers/clk/rockchip/clk.h
>> index 2194ffa..51e797c 100644
>> --- a/drivers/clk/rockchip/clk.h
>> +++ b/drivers/clk/rockchip/clk.h
>> @@ -618,16 +618,14 @@ void rockchip_clk_protect_critical(const char
>> *const clocks[], int nclocks);
>> void rockchip_register_restart_notifier(struct rockchip_clk_provider
>> *ctx,
>> unsigned int reg, void (*cb)(void));
>>
>> -#define ROCKCHIP_SOFTRST_HIWORD_MASK BIT(0)
>> -
>> #ifdef CONFIG_RESET_CONTROLLER
>> void rockchip_register_softrst(struct device_node *np,
>> unsigned int num_regs,
>> - void __iomem *base, u8 flags);
>> + void __iomem *base);
>> #else
>> static inline void rockchip_register_softrst(struct device_node *np,
>> unsigned int num_regs,
>> - void __iomem *base, u8 flags)
>> + void __iomem *base)
>> {
>> }
>> #endif
>> diff --git a/drivers/clk/rockchip/softrst.c
>> b/drivers/clk/rockchip/softrst.c
>> index 21218987..d6b865f 100644
>> --- a/drivers/clk/rockchip/softrst.c
>> +++ b/drivers/clk/rockchip/softrst.c
>> @@ -24,8 +24,6 @@ struct rockchip_softrst {
>> void __iomem *reg_base;
>> int num_regs;
>> int num_per_reg;
>> - u8 flags;
>> - spinlock_t lock;
>> };
>>
>> static int rockchip_softrst_assert(struct reset_controller_dev *rcdev,
>> @@ -37,20 +35,8 @@ static int rockchip_softrst_assert(struct
>> reset_controller_dev *rcdev,
>> int bank = id / softrst->num_per_reg;
>> int offset = id % softrst->num_per_reg;
>>
>> - if (softrst->flags& ROCKCHIP_SOFTRST_HIWORD_MASK) {
>> - writel(BIT(offset) | (BIT(offset)<< 16),
>> - softrst->reg_base + (bank * 4));
>> - } else {
>> - unsigned long flags;
>> - u32 reg;
>> -
>> - spin_lock_irqsave(&softrst->lock, flags);
>> -
>> - reg = readl(softrst->reg_base + (bank * 4));
>> - writel(reg | BIT(offset), softrst->reg_base + (bank * 4));
>> -
>> - spin_unlock_irqrestore(&softrst->lock, flags);
>> - }
>> + writel(BIT(offset) | (BIT(offset)<< 16),
>> + softrst->reg_base + (bank * 4));
>>
>> return 0;
>> }
>> @@ -64,19 +50,7 @@ static int rockchip_softrst_deassert(struct
>> reset_controller_dev *rcdev,
>> int bank = id / softrst->num_per_reg;
>> int offset = id % softrst->num_per_reg;
>>
>> - if (softrst->flags& ROCKCHIP_SOFTRST_HIWORD_MASK) {
>> - writel((BIT(offset)<< 16), softrst->reg_base + (bank * 4));
>> - } else {
>> - unsigned long flags;
>> - u32 reg;
>> -
>> - spin_lock_irqsave(&softrst->lock, flags);
>> -
>> - reg = readl(softrst->reg_base + (bank * 4));
>> - writel(reg& ~BIT(offset), softrst->reg_base + (bank * 4));
>> -
>> - spin_unlock_irqrestore(&softrst->lock, flags);
>> - }
>> + writel((BIT(offset)<< 16), softrst->reg_base + (bank * 4));
>>
>> return 0;
>> }
>> @@ -88,7 +62,7 @@ static const struct reset_control_ops
>> rockchip_softrst_ops = {
>>
>> void __init rockchip_register_softrst(struct device_node *np,
>> unsigned int num_regs,
>> - void __iomem *base, u8 flags)
>> + void __iomem *base)
>> {
>> struct rockchip_softrst *softrst;
>> int ret;
>> @@ -97,13 +71,9 @@ void __init rockchip_register_softrst(struct
>> device_node *np,
>> if (!softrst)
>> return;
>>
>> - spin_lock_init(&softrst->lock);
>> -
>> softrst->reg_base = base;
>> - softrst->flags = flags;
>> softrst->num_regs = num_regs;
>> - softrst->num_per_reg = (flags& ROCKCHIP_SOFTRST_HIWORD_MASK) ? 16
>> - : 32;
>> + softrst->num_per_reg = 16;
>>
>> softrst->rcdev.owner = THIS_MODULE;
>> softrst->rcdev.nr_resets = num_regs * softrst->num_per_reg;
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] clk: rockchip: remove ROCKCHIP_SOFTRST_HIWORD_MASK for rockchip platform
2016-06-03 3:35 ` Shawn Lin
@ 2016-06-03 7:29 ` Heiko Stübner
2016-06-03 7:38 ` Shawn Lin
0 siblings, 1 reply; 5+ messages in thread
From: Heiko Stübner @ 2016-06-03 7:29 UTC (permalink / raw)
To: Shawn Lin
Cc: Xing Zheng, Shawn Lin, linux-rockchip, Stephen Boyd,
linux-kernel, linux-clk
Hi Shawn,
Am Freitag, 3. Juni 2016, 11:35:32 schrieb Shawn Lin:
> 在 2016/6/3 9:25, Xing Zheng 写道:
> > On 2016年06月03日 08:54, Shawn Lin wrote:
> >> I check all the Socs including RK2928/3000/3066/3028X/316X/312X/
> >> 3190/3188/3228/3368/3399/3036, and find all of them use high 16-bit
> >> as write mask. Obviously we don't need ROCKCHIP_SOFTRST_HIWORD_MASK
> >> any more(actually I don't know why we need it before). This patch
> >> removes it to simplify the code and save a little cpu cycle when calling
> >> assert or deassert callback.
> >
> > In my opinion, this flag can be used for compatibility, we can not
> > ensure that our SoCs will not use the 32bit SOFTRST_CONs in future.
> >
> > Thanks.
>
> Thanks for sharing your thought.
>
> I'm not 100% sure, but I'm 99% sure about we won't let
> it happened. You have to consider the backward compatibility
> rather than the future ones. If you got a chip with 10 bit, or
> 8bit for SOFTRST_CONX, so how do you wanna deal with it?
> Should we now add ROCKCHIP_SOFTRST_X_BIT_MASK? :)
older SoCs like the rk2818 (ARM9-based) do actually use 32bit softrst
registers. And if I ever get my hands on one of those, I'd actually try to
support it :-) .
So I'd really like to keep the flag.
Heiko
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] clk: rockchip: remove ROCKCHIP_SOFTRST_HIWORD_MASK for rockchip platform
2016-06-03 7:29 ` Heiko Stübner
@ 2016-06-03 7:38 ` Shawn Lin
0 siblings, 0 replies; 5+ messages in thread
From: Shawn Lin @ 2016-06-03 7:38 UTC (permalink / raw)
To: Heiko Stübner
Cc: shawn.lin, Xing Zheng, linux-rockchip, Stephen Boyd,
linux-kernel, linux-clk
在 2016/6/3 15:29, Heiko Stübner 写道:
> Hi Shawn,
>
> Am Freitag, 3. Juni 2016, 11:35:32 schrieb Shawn Lin:
>> 在 2016/6/3 9:25, Xing Zheng 写道:
>>> On 2016年06月03日 08:54, Shawn Lin wrote:
>>>> I check all the Socs including RK2928/3000/3066/3028X/316X/312X/
>>>> 3190/3188/3228/3368/3399/3036, and find all of them use high 16-bit
>>>> as write mask. Obviously we don't need ROCKCHIP_SOFTRST_HIWORD_MASK
>>>> any more(actually I don't know why we need it before). This patch
>>>> removes it to simplify the code and save a little cpu cycle when calling
>>>> assert or deassert callback.
>>>
>>> In my opinion, this flag can be used for compatibility, we can not
>>> ensure that our SoCs will not use the 32bit SOFTRST_CONs in future.
>>>
>>> Thanks.
>>
>> Thanks for sharing your thought.
>>
>> I'm not 100% sure, but I'm 99% sure about we won't let
>> it happened. You have to consider the backward compatibility
>> rather than the future ones. If you got a chip with 10 bit, or
>> 8bit for SOFTRST_CONX, so how do you wanna deal with it?
>> Should we now add ROCKCHIP_SOFTRST_X_BIT_MASK? :)
>
> older SoCs like the rk2818 (ARM9-based) do actually use 32bit softrst
> registers. And if I ever get my hands on one of those, I'd actually try to
> support it :-) .
>
> So I'd really like to keep the flag.
okay, fair engough. I just forgot to check rk281x and rk2808 etc, which
seems too old for me. Yes, IIRC it did use 32bit softrst...
So this flag should be used to keep the backward compatibility
if someday someone wanna upstream something for rk2818.. :)
please drop this patch.
>
> Heiko
>
>
>
--
Best Regards
Shawn Lin
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-06-03 7:39 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-03 0:54 [PATCH 2/2] clk: rockchip: remove ROCKCHIP_SOFTRST_HIWORD_MASK for rockchip platform Shawn Lin
2016-06-03 1:25 ` Xing Zheng
2016-06-03 3:35 ` Shawn Lin
2016-06-03 7:29 ` Heiko Stübner
2016-06-03 7:38 ` Shawn Lin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).