Re: [PATCH v2] clk: sunxi: predivider handling for factors clock

From: Chen-Yu Tsai
Date: Mon Apr 25 2016 - 10:52:29 EST


Hi,

On Wed, Apr 20, 2016 at 12:47 AM, Vishnu Patekar
<vishnupatekar0510@xxxxxxxxx> 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@xxxxxxxxx>
> ---
> 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