lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Mon, 25 Apr 2016 22:51:59 +0800
From:	Chen-Yu Tsai <wens@...e.org>
To:	Vishnu Patekar <vishnupatekar0510@...il.com>
Cc:	Maxime Ripard <maxime.ripard@...e-electrons.com>,
	Emilio Lopez <emilio@...pez.com.ar>,
	Chen-Yu Tsai <wens@...e.org>,
	Stephen Boyd <sboyd@...eaurora.org>,
	linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	linux-sunxi <linux-sunxi@...glegroups.com>
Subject: Re: [PATCH v2] clk: sunxi: predivider handling for factors clock

Hi,

On Wed, Apr 20, 2016 at 12:47 AM, Vishnu Patekar
<vishnupatekar0510@...il.com> wrote:
> For A31 ahb1 and a83t ahb1 clocks have predivider for certain parent.
> To handle this, this patch adds predivider table with parent index,
> prediv shift and width, parents with predivider will have nonzero width.
>
> Rate adjustment is moved from clock specific recalc function to generic
> factors recalc. Also, adds prediv table for a31.
>
> Signed-off-by: Vishnu Patekar <vishnupatekar0510@...il.com>
> ---
>  drivers/clk/sunxi/clk-factors.c | 31 +++++++++++++++----------------
>  drivers/clk/sunxi/clk-factors.h | 10 +++++++++-
>  drivers/clk/sunxi/clk-sunxi.c   | 31 +++++++++----------------------
>  3 files changed, 33 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/clk/sunxi/clk-factors.c b/drivers/clk/sunxi/clk-factors.c
> index ddefe96..8f3b637 100644
> --- a/drivers/clk/sunxi/clk-factors.c
> +++ b/drivers/clk/sunxi/clk-factors.c
> @@ -45,10 +45,12 @@ static unsigned long clk_factors_recalc_rate(struct clk_hw *hw,
>                                              unsigned long parent_rate)
>  {
>         u8 n = 1, k = 0, p = 0, m = 0;
> +       u8 par_index = 0;
>         u32 reg;
>         unsigned long rate;
>         struct clk_factors *factors = to_clk_factors(hw);
>         const struct clk_factors_config *config = factors->config;
> +       const struct clk_factors_prediv *prediv = factors->prediv_config;
>
>         /* Fetch the register value */
>         reg = readl(factors->reg);
> @@ -63,24 +65,16 @@ static unsigned long clk_factors_recalc_rate(struct clk_hw *hw,
>         if (config->pwidth != SUNXI_FACTORS_NOT_APPLICABLE)
>                 p = FACTOR_GET(config->pshift, config->pwidth, reg);
>
> -       if (factors->recalc) {
> -               struct factors_request factors_req = {
> -                       .parent_rate = parent_rate,
> -                       .n = n,
> -                       .k = k,
> -                       .m = m,
> -                       .p = p,
> -               };
> -
> +       if (prediv) {
>                 /* get mux details from mux clk structure */
>                 if (factors->mux)
> -                       factors_req.parent_index =
> -                               (reg >> factors->mux->shift) &
> -                               factors->mux->mask;
> -
> -               factors->recalc(&factors_req);
> +                       par_index = (reg >> factors->mux->shift) &
> +                                       factors->mux->mask;
>
> -               return factors_req.rate;
> +               if (prediv[par_index].width != SUNXI_FACTORS_NOT_APPLICABLE) {
> +                       m = FACTOR_GET(prediv[par_index].shift,
> +                               prediv[par_index].width, reg);
> +               }
>         }
>
>         /* Calculate the rate */
> @@ -102,8 +96,12 @@ static int clk_factors_determine_rate(struct clk_hw *hw,
>         for (i = 0; i < num_parents; i++) {
>                 struct factors_request factors_req = {
>                         .rate = req->rate,
> -                       .parent_index = i,
>                 };
> +
> +               if (factors->prediv_config)
> +                       factors_req.prediv_width =
> +                                               factors->prediv_config[i].width;
> +
>                 parent = clk_hw_get_parent_by_index(hw, i);
>                 if (!parent)
>                         continue;
> @@ -211,6 +209,7 @@ struct clk *sunxi_factors_register(struct device_node *node,
>         /* set up factors properties */
>         factors->reg = reg;
>         factors->config = data->table;
> +       factors->prediv_config = data->prediv_table;
>         factors->get_factors = data->getter;
>         factors->recalc = data->recalc;
>         factors->lock = lock;
> diff --git a/drivers/clk/sunxi/clk-factors.h b/drivers/clk/sunxi/clk-factors.h
> index 1e63c5b..b1b7745 100644
> --- a/drivers/clk/sunxi/clk-factors.h
> +++ b/drivers/clk/sunxi/clk-factors.h
> @@ -18,10 +18,16 @@ struct clk_factors_config {
>         u8 n_start;
>  };
>
> +struct clk_factors_prediv {
> +       u8 parent_index;
> +       u8 shift;
> +       u8 width;
> +};
> +
>  struct factors_request {
>         unsigned long rate;
>         unsigned long parent_rate;
> -       u8 parent_index;
> +       u8 prediv_width;
>         u8 n;
>         u8 k;
>         u8 m;
> @@ -33,6 +39,7 @@ struct factors_data {
>         int mux;
>         int muxmask;
>         const struct clk_factors_config *table;
> +       const struct clk_factors_prediv *prediv_table;
>         void (*getter)(struct factors_request *req);
>         void (*recalc)(struct factors_request *req);

You removed usage of this callback. Please remove it from the data structures
as well, so no one assumes it's still applicable.

>         const char *name;
> @@ -42,6 +49,7 @@ struct clk_factors {
>         struct clk_hw hw;
>         void __iomem *reg;
>         const struct clk_factors_config *config;
> +       const struct clk_factors_prediv *prediv_config;
>         void (*get_factors)(struct factors_request *req);
>         void (*recalc)(struct factors_request *req);
>         spinlock_t *lock;
> diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
> index 91de0a0..5a5f26b 100644
> --- a/drivers/clk/sunxi/clk-sunxi.c
> +++ b/drivers/clk/sunxi/clk-sunxi.c
> @@ -282,8 +282,6 @@ static void sun5i_a13_get_ahb_factors(struct factors_request *req)
>         req->p = div;
>  }
>
> -#define SUN6I_AHB1_PARENT_PLL6 3
> -
>  /**
>   * sun6i_a31_get_ahb_factors() - calculates m, p factors for AHB
>   * AHB rate is calculated as follows
> @@ -307,7 +305,7 @@ static void sun6i_get_ahb1_factors(struct factors_request *req)
>         div = DIV_ROUND_UP(req->parent_rate, req->rate);
>
>         /* calculate pre-divider if parent is pll6 */
> -       if (req->parent_index == SUN6I_AHB1_PARENT_PLL6) {
> +       if (req->prediv_width) {
>                 if (div < 4)
>                         calcp = 0;
>                 else if (div / 2 < 4)
> @@ -329,22 +327,6 @@ static void sun6i_get_ahb1_factors(struct factors_request *req)
>  }
>
>  /**
> - * sun6i_ahb1_recalc() - calculates AHB clock rate from m, p factors and
> - *                      parent index
> - */
> -static void sun6i_ahb1_recalc(struct factors_request *req)
> -{
> -       req->rate = req->parent_rate;
> -
> -       /* apply pre-divider first if parent is pll6 */
> -       if (req->parent_index == SUN6I_AHB1_PARENT_PLL6)
> -               req->rate /= req->m + 1;
> -
> -       /* clk divider */
> -       req->rate >>= req->p;
> -}
> -
> -/**
>   * sun4i_get_apb1_factors() - calculates m, p factors for APB1
>   * APB1 rate is calculated as follows
>   * rate = (parent_rate >> p) / (m + 1);
> @@ -474,12 +456,17 @@ static const struct clk_factors_config sun5i_a13_ahb_config = {
>  };
>
>  static const struct clk_factors_config sun6i_ahb1_config = {
> -       .mshift = 6,
> -       .mwidth = 2,
>         .pshift = 4,
>         .pwidth = 2,
>  };
>
> +static const struct clk_factors_prediv sun6i_ahb1_prediv[] = {
> +       {.parent_index = 0, .shift = 0, .width = 0 },   /* LOSC  */
> +       {.parent_index = 1, .shift = 0, .width = 0 },   /* OSC24MHz */
> +       {.parent_index = 2, .shift = 0, .width = 0 },   /* AXI */
> +       {.parent_index = 3, .shift = 6, .width = 2 }    /* PLL6/Pre_div */

           ^ a space here would be nice.                         ^
spaces around here too.

> +};
> +
>  static const struct clk_factors_config sun4i_apb1_config = {
>         .mshift = 0,
>         .mwidth = 5,
> @@ -551,8 +538,8 @@ static const struct factors_data sun6i_ahb1_data __initconst = {
>         .mux = 12,
>         .muxmask = BIT(1) | BIT(0),
>         .table = &sun6i_ahb1_config,
> +       .prediv_table = sun6i_ahb1_prediv,
>         .getter = sun6i_get_ahb1_factors,
> -       .recalc = sun6i_ahb1_recalc,
>  };
>
>  static const struct factors_data sun4i_apb1_data __initconst = {
> --
> 1.9.1
>

The rest looks good.

Regards
ChenYu

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ