| Message ID | 20200414200017.226136-3-martin.blumenstingl@googlemail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Neil Armstrong |
| Headers | show |
| Series | clk: meson8b: updates for video clocks / resets | expand |
On Tue 14 Apr 2020 at 22:00, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote: > CLKC_RESET_VID_DIVIDER_CNTL_RESET_N_POST and > CLKC_RESET_VID_DIVIDER_CNTL_RESET_N_PRE are active low. This means: > - asserting them requires setting the register value to 0 > - de-asserting them requires setting the register value to 1 > > Set the register value accordingly for these two reset lines by setting > the inverted the register value compared to all other reset lines. > > Fixes: 189621726bc2f6 ("clk: meson: meson8b: register the built-in reset controller") > Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> > --- > drivers/clk/meson/meson8b.c | 81 ++++++++++++++++++++++++++----------- > 1 file changed, 58 insertions(+), 23 deletions(-) > > diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c > index 90d284ffc780..fa251e45e208 100644 > --- a/drivers/clk/meson/meson8b.c > +++ b/drivers/clk/meson/meson8b.c > @@ -3506,54 +3506,87 @@ static struct clk_regmap *const meson8b_clk_regmaps[] = { > static const struct meson8b_clk_reset_line { > u32 reg; > u8 bit_idx; > + bool active_low; > } meson8b_clk_reset_bits[] = { > [CLKC_RESET_L2_CACHE_SOFT_RESET] = { > - .reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 30 > + .reg = HHI_SYS_CPU_CLK_CNTL0, > + .bit_idx = 30, > + .active_low = false, > }, > [CLKC_RESET_AXI_64_TO_128_BRIDGE_A5_SOFT_RESET] = { > - .reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 29 > + .reg = HHI_SYS_CPU_CLK_CNTL0, > + .bit_idx = 29, > + .active_low = false, > }, > [CLKC_RESET_SCU_SOFT_RESET] = { > - .reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 28 > + .reg = HHI_SYS_CPU_CLK_CNTL0, > + .bit_idx = 28, > + .active_low = false, > }, > [CLKC_RESET_CPU3_SOFT_RESET] = { > - .reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 27 > + .reg = HHI_SYS_CPU_CLK_CNTL0, > + .bit_idx = 27, > + .active_low = false, > }, > [CLKC_RESET_CPU2_SOFT_RESET] = { > - .reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 26 > + .reg = HHI_SYS_CPU_CLK_CNTL0, > + .bit_idx = 26, > + .active_low = false, > }, > [CLKC_RESET_CPU1_SOFT_RESET] = { > - .reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 25 > + .reg = HHI_SYS_CPU_CLK_CNTL0, > + .bit_idx = 25, > + .active_low = false, > }, > [CLKC_RESET_CPU0_SOFT_RESET] = { > - .reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 24 > + .reg = HHI_SYS_CPU_CLK_CNTL0, > + .bit_idx = 24, > + .active_low = false, > }, > [CLKC_RESET_A5_GLOBAL_RESET] = { > - .reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 18 > + .reg = HHI_SYS_CPU_CLK_CNTL0, > + .bit_idx = 18, > + .active_low = false, > }, > [CLKC_RESET_A5_AXI_SOFT_RESET] = { > - .reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 17 > + .reg = HHI_SYS_CPU_CLK_CNTL0, > + .bit_idx = 17, > + .active_low = false, > }, > [CLKC_RESET_A5_ABP_SOFT_RESET] = { > - .reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 16 > + .reg = HHI_SYS_CPU_CLK_CNTL0, > + .bit_idx = 16, > + .active_low = false, > }, > [CLKC_RESET_AXI_64_TO_128_BRIDGE_MMC_SOFT_RESET] = { > - .reg = HHI_SYS_CPU_CLK_CNTL1, .bit_idx = 30 > + .reg = HHI_SYS_CPU_CLK_CNTL1, > + .bit_idx = 30, > + .active_low = false, > }, > [CLKC_RESET_VID_CLK_CNTL_SOFT_RESET] = { > - .reg = HHI_VID_CLK_CNTL, .bit_idx = 15 > + .reg = HHI_VID_CLK_CNTL, > + .bit_idx = 15, > + .active_low = false, > }, > [CLKC_RESET_VID_DIVIDER_CNTL_SOFT_RESET_POST] = { > - .reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 7 > + .reg = HHI_VID_DIVIDER_CNTL, > + .bit_idx = 7, > + .active_low = false, > }, > [CLKC_RESET_VID_DIVIDER_CNTL_SOFT_RESET_PRE] = { > - .reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 3 > + .reg = HHI_VID_DIVIDER_CNTL, > + .bit_idx = 3, > + .active_low = false, > }, > [CLKC_RESET_VID_DIVIDER_CNTL_RESET_N_POST] = { > - .reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 1 > + .reg = HHI_VID_DIVIDER_CNTL, > + .bit_idx = 1, > + .active_low = true, > }, > [CLKC_RESET_VID_DIVIDER_CNTL_RESET_N_PRE] = { > - .reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 0 > + .reg = HHI_VID_DIVIDER_CNTL, > + .bit_idx = 0, > + .active_low = true, > }, > }; > > @@ -3562,22 +3595,24 @@ static int meson8b_clk_reset_update(struct reset_controller_dev *rcdev, > { > struct meson8b_clk_reset *meson8b_clk_reset = > container_of(rcdev, struct meson8b_clk_reset, reset); > - unsigned long flags; > const struct meson8b_clk_reset_line *reset; > + unsigned long flags; > + unsigned int value; suggestion: unsigned int value = 0; > > if (id >= ARRAY_SIZE(meson8b_clk_reset_bits)) > return -EINVAL; > > reset = &meson8b_clk_reset_bits[id]; > > + if (assert == reset->active_low) > + value = 0; > + else > + value = BIT(reset->bit_idx); if (assert ^ reset->active_low) value = BIT(reset->bit_idx); ? > + > spin_lock_irqsave(&meson_clk_lock, flags); > > - if (assert) > - regmap_update_bits(meson8b_clk_reset->regmap, reset->reg, > - BIT(reset->bit_idx), BIT(reset->bit_idx)); > - else > - regmap_update_bits(meson8b_clk_reset->regmap, reset->reg, > - BIT(reset->bit_idx), 0); > + regmap_update_bits(meson8b_clk_reset->regmap, reset->reg, > + BIT(reset->bit_idx), value); > > spin_unlock_irqrestore(&meson_clk_lock, flags);
Hi Jerome, On Thu, Apr 16, 2020 at 12:38 PM Jerome Brunet <jbrunet@baylibre.com> wrote: [...] > > > > if (id >= ARRAY_SIZE(meson8b_clk_reset_bits)) > > return -EINVAL; > > > > reset = &meson8b_clk_reset_bits[id]; > > > > + if (assert == reset->active_low) > > + value = 0; > > + else > > + value = BIT(reset->bit_idx); > > if (assert ^ reset->active_low) > value = BIT(reset->bit_idx); I can do that, but I prefer "!=" over "^" because the result is expected to be a bool (and because I'm not used to reading "^" for logical comparisons) will this work for you as well? Martin
On Thu 16 Apr 2020 at 20:12, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote: > Hi Jerome, > > On Thu, Apr 16, 2020 at 12:38 PM Jerome Brunet <jbrunet@baylibre.com> wrote: > [...] >> > >> > if (id >= ARRAY_SIZE(meson8b_clk_reset_bits)) >> > return -EINVAL; >> > >> > reset = &meson8b_clk_reset_bits[id]; >> > >> > + if (assert == reset->active_low) >> > + value = 0; >> > + else >> > + value = BIT(reset->bit_idx); >> >> if (assert ^ reset->active_low) >> value = BIT(reset->bit_idx); > I can do that, but I prefer "!=" over "^" because the result is > expected to be a bool (and because I'm not used to reading "^" for > logical comparisons) > will this work for you as well? yes > > > Martin
diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c index 90d284ffc780..fa251e45e208 100644 --- a/drivers/clk/meson/meson8b.c +++ b/drivers/clk/meson/meson8b.c @@ -3506,54 +3506,87 @@ static struct clk_regmap *const meson8b_clk_regmaps[] = { static const struct meson8b_clk_reset_line { u32 reg; u8 bit_idx; + bool active_low; } meson8b_clk_reset_bits[] = { [CLKC_RESET_L2_CACHE_SOFT_RESET] = { - .reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 30 + .reg = HHI_SYS_CPU_CLK_CNTL0, + .bit_idx = 30, + .active_low = false, }, [CLKC_RESET_AXI_64_TO_128_BRIDGE_A5_SOFT_RESET] = { - .reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 29 + .reg = HHI_SYS_CPU_CLK_CNTL0, + .bit_idx = 29, + .active_low = false, }, [CLKC_RESET_SCU_SOFT_RESET] = { - .reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 28 + .reg = HHI_SYS_CPU_CLK_CNTL0, + .bit_idx = 28, + .active_low = false, }, [CLKC_RESET_CPU3_SOFT_RESET] = { - .reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 27 + .reg = HHI_SYS_CPU_CLK_CNTL0, + .bit_idx = 27, + .active_low = false, }, [CLKC_RESET_CPU2_SOFT_RESET] = { - .reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 26 + .reg = HHI_SYS_CPU_CLK_CNTL0, + .bit_idx = 26, + .active_low = false, }, [CLKC_RESET_CPU1_SOFT_RESET] = { - .reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 25 + .reg = HHI_SYS_CPU_CLK_CNTL0, + .bit_idx = 25, + .active_low = false, }, [CLKC_RESET_CPU0_SOFT_RESET] = { - .reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 24 + .reg = HHI_SYS_CPU_CLK_CNTL0, + .bit_idx = 24, + .active_low = false, }, [CLKC_RESET_A5_GLOBAL_RESET] = { - .reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 18 + .reg = HHI_SYS_CPU_CLK_CNTL0, + .bit_idx = 18, + .active_low = false, }, [CLKC_RESET_A5_AXI_SOFT_RESET] = { - .reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 17 + .reg = HHI_SYS_CPU_CLK_CNTL0, + .bit_idx = 17, + .active_low = false, }, [CLKC_RESET_A5_ABP_SOFT_RESET] = { - .reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 16 + .reg = HHI_SYS_CPU_CLK_CNTL0, + .bit_idx = 16, + .active_low = false, }, [CLKC_RESET_AXI_64_TO_128_BRIDGE_MMC_SOFT_RESET] = { - .reg = HHI_SYS_CPU_CLK_CNTL1, .bit_idx = 30 + .reg = HHI_SYS_CPU_CLK_CNTL1, + .bit_idx = 30, + .active_low = false, }, [CLKC_RESET_VID_CLK_CNTL_SOFT_RESET] = { - .reg = HHI_VID_CLK_CNTL, .bit_idx = 15 + .reg = HHI_VID_CLK_CNTL, + .bit_idx = 15, + .active_low = false, }, [CLKC_RESET_VID_DIVIDER_CNTL_SOFT_RESET_POST] = { - .reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 7 + .reg = HHI_VID_DIVIDER_CNTL, + .bit_idx = 7, + .active_low = false, }, [CLKC_RESET_VID_DIVIDER_CNTL_SOFT_RESET_PRE] = { - .reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 3 + .reg = HHI_VID_DIVIDER_CNTL, + .bit_idx = 3, + .active_low = false, }, [CLKC_RESET_VID_DIVIDER_CNTL_RESET_N_POST] = { - .reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 1 + .reg = HHI_VID_DIVIDER_CNTL, + .bit_idx = 1, + .active_low = true, }, [CLKC_RESET_VID_DIVIDER_CNTL_RESET_N_PRE] = { - .reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 0 + .reg = HHI_VID_DIVIDER_CNTL, + .bit_idx = 0, + .active_low = true, }, }; @@ -3562,22 +3595,24 @@ static int meson8b_clk_reset_update(struct reset_controller_dev *rcdev, { struct meson8b_clk_reset *meson8b_clk_reset = container_of(rcdev, struct meson8b_clk_reset, reset); - unsigned long flags; const struct meson8b_clk_reset_line *reset; + unsigned long flags; + unsigned int value; if (id >= ARRAY_SIZE(meson8b_clk_reset_bits)) return -EINVAL; reset = &meson8b_clk_reset_bits[id]; + if (assert == reset->active_low) + value = 0; + else + value = BIT(reset->bit_idx); + spin_lock_irqsave(&meson_clk_lock, flags); - if (assert) - regmap_update_bits(meson8b_clk_reset->regmap, reset->reg, - BIT(reset->bit_idx), BIT(reset->bit_idx)); - else - regmap_update_bits(meson8b_clk_reset->regmap, reset->reg, - BIT(reset->bit_idx), 0); + regmap_update_bits(meson8b_clk_reset->regmap, reset->reg, + BIT(reset->bit_idx), value); spin_unlock_irqrestore(&meson_clk_lock, flags);
CLKC_RESET_VID_DIVIDER_CNTL_RESET_N_POST and CLKC_RESET_VID_DIVIDER_CNTL_RESET_N_PRE are active low. This means: - asserting them requires setting the register value to 0 - de-asserting them requires setting the register value to 1 Set the register value accordingly for these two reset lines by setting the inverted the register value compared to all other reset lines. Fixes: 189621726bc2f6 ("clk: meson: meson8b: register the built-in reset controller") Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> --- drivers/clk/meson/meson8b.c | 81 ++++++++++++++++++++++++++----------- 1 file changed, 58 insertions(+), 23 deletions(-)