linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: maxime.ripard@free-electrons.com (Maxime Ripard)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 02/16] clk: sunxi-ng: Add common infrastructure
Date: Sun, 15 May 2016 20:31:22 +0200	[thread overview]
Message-ID: <20160515183122.GA27618@lukather> (raw)
In-Reply-To: <CAGb2v67XWt6tjG_xXWgGatTaiN5uYQb2_Q6bgixMDUkHfVHLHg@mail.gmail.com>

Hi,

On Mon, May 09, 2016 at 06:01:45PM +0800, Chen-Yu Tsai wrote:
> On Mon, May 9, 2016 at 4:01 AM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > Start our new clock infrastructure by adding the registration code, common
> > structure and common code.
> >
> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > ---
> >  drivers/clk/Makefile              |   1 +
> >  drivers/clk/sunxi-ng/Makefile     |   2 +
> >  drivers/clk/sunxi-ng/ccu_common.c | 108 ++++++++++++++++++++++++++++++++++++++
> >  drivers/clk/sunxi-ng/ccu_common.h |  74 ++++++++++++++++++++++++++
> >  drivers/clk/sunxi-ng/ccu_factor.h |  15 ++++++
> >  drivers/clk/sunxi-ng/ccu_mux.h    |  20 +++++++
> >  drivers/clk/sunxi-ng/ccu_reset.c  |  55 +++++++++++++++++++
> >  drivers/clk/sunxi-ng/ccu_reset.h  |  40 ++++++++++++++
> >  8 files changed, 315 insertions(+)
> >  create mode 100644 drivers/clk/sunxi-ng/Makefile
> >  create mode 100644 drivers/clk/sunxi-ng/ccu_common.c
> >  create mode 100644 drivers/clk/sunxi-ng/ccu_common.h
> >  create mode 100644 drivers/clk/sunxi-ng/ccu_factor.h
> >  create mode 100644 drivers/clk/sunxi-ng/ccu_mux.h
> >  create mode 100644 drivers/clk/sunxi-ng/ccu_reset.c
> >  create mode 100644 drivers/clk/sunxi-ng/ccu_reset.h
> >
> > diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> > index 4ef71a13ab37..83a93cd9e21d 100644
> > --- a/drivers/clk/Makefile
> > +++ b/drivers/clk/Makefile
> > @@ -78,6 +78,7 @@ obj-$(CONFIG_ARCH_SOCFPGA)            += socfpga/
> >  obj-$(CONFIG_PLAT_SPEAR)               += spear/
> >  obj-$(CONFIG_ARCH_STI)                 += st/
> >  obj-$(CONFIG_ARCH_SUNXI)               += sunxi/
> > +obj-$(CONFIG_ARCH_SUNXI)               += sunxi-ng/
> >  obj-$(CONFIG_ARCH_TEGRA)               += tegra/
> >  obj-y                                  += ti/
> >  obj-$(CONFIG_ARCH_U8500)               += ux500/
> > diff --git a/drivers/clk/sunxi-ng/Makefile b/drivers/clk/sunxi-ng/Makefile
> > new file mode 100644
> > index 000000000000..bd3461b0f38c
> > --- /dev/null
> > +++ b/drivers/clk/sunxi-ng/Makefile
> > @@ -0,0 +1,2 @@
> > +obj-y += ccu_common.o
> > +obj-y += ccu_reset.o
> > diff --git a/drivers/clk/sunxi-ng/ccu_common.c b/drivers/clk/sunxi-ng/ccu_common.c
> > new file mode 100644
> > index 000000000000..1d9242566fbd
> > --- /dev/null
> > +++ b/drivers/clk/sunxi-ng/ccu_common.c
> > @@ -0,0 +1,108 @@
> > +/*
> > + * Copyright 2016 Maxime Ripard
> > + *
> > + * Maxime Ripard <maxime.ripard@free-electrons.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#include <linux/clk-provider.h>
> > +#include <linux/iopoll.h>
> > +#include <linux/of_address.h>
> > +#include <linux/slab.h>
> > +
> > +#include "ccu_common.h"
> > +#include "ccu_reset.h"
> > +
> > +static DEFINE_SPINLOCK(ccu_lock);
> > +
> > +void ccu_helper_wait_for_lock(struct ccu_common *common, u32 lock)
> > +{
> > +       u32 reg;
> > +
> > +       if (!(common->features & CCU_FEATURE_LOCK))
> > +               return;
> > +
> > +       WARN_ON(readl_relaxed_poll_timeout(common->base + common->reg, reg,
> > +                                          !(reg & lock), 0, 500));
> 
>                                     no delay between reads? ^

Yes, I intended it to be a simple busy waiting loop since I don't
expect it to be very long. Do yu have any more data on how much time
it usually takes?

> 
> > +int sunxi_ccu_probe(struct device_node *node,
> > +                   const struct sunxi_ccu_desc *desc)
> > +{
> > +       struct ccu_common **cclks = desc->clks;
> > +       size_t num_clks = desc->num_clks;
> > +       struct clk_onecell_data *data;
> > +       struct ccu_reset *reset;
> > +       struct clk **clks;
> > +       void __iomem *reg;
> > +       int i, ret;
> > +
> > +       reg = of_iomap(node, 0);
> 
> Why not of_io_request_and_map?

Because initially I still had some old clocks that were probing, which
was leading to some issues. This is obviously not the case anymore,
I'll switch to it.

> 
> > +       if (IS_ERR(reg)) {
> 
> And of_iomap returns NULL on error. This is for of_io_request_and_map.
> 
> > +               pr_err("%s: Could not map the clock registers\n",
> > +                      of_node_full_name(node));
> > +               return PTR_ERR(reg);
> > +       }
> > +
> > +       data = kzalloc(sizeof(*data), GFP_KERNEL);
> > +       if (!data)
> > +               return -ENOMEM;
> > +
> > +       clks = kcalloc(num_clks, sizeof(struct clk *), GFP_KERNEL);
> > +       if (!clks)
> > +               return -ENOMEM;
> > +
> > +       data->clks = clks;
> > +       data->clk_num = num_clks;
> > +
> > +       for (i = 0; i < num_clks; i++) {
> > +               struct ccu_common *cclk = cclks[i];
> > +               struct clk *clk;
> > +
> > +               if (!cclk) {
> > +                       cclk = ERR_PTR(-ENOENT);
> 
> This seems redundant, unless you intended to use it elsewhere?

Yeah, that was supposed to be clks[i] = ERR_PTR(..);

I'll fix it.

> 
> > +                       continue;
> > +               }
> > +
> > +               cclk->base = reg;
> > +               cclk->lock = &ccu_lock;
> > +
> > +               clk = clk_register(NULL, &cclk->hw);
> > +               if (IS_ERR(clk))
> > +                       continue;
> > +
> > +               clks[i] = clk;
> > +       }
> > +
> > +       ret = of_clk_add_provider(node, of_clk_src_onecell_get, data);
> > +       if (ret)
> > +               goto err_clk_unreg;
> > +
> > +       reset = kzalloc(sizeof(*reset), GFP_KERNEL);
> > +       reset->rcdev.of_node = node;
> > +       reset->rcdev.ops = &ccu_reset_ops;
> > +       reset->rcdev.owner = THIS_MODULE;
> > +       reset->rcdev.nr_resets = desc->num_resets;
> > +       reset->base = reg;
> > +       reset->lock = &ccu_lock;
> > +       reset->reset_map = desc->resets;
> > +
> > +       ret = reset_controller_register(&reset->rcdev);
> > +       if (ret)
> > +               goto err_of_clk_unreg;
> > +
> > +       return 0;
> > +
> > +err_of_clk_unreg:
> > +err_clk_unreg:
> > +       return ret;
> > +}
> > diff --git a/drivers/clk/sunxi-ng/ccu_common.h b/drivers/clk/sunxi-ng/ccu_common.h
> > new file mode 100644
> > index 000000000000..e8b477fcd320
> > --- /dev/null
> > +++ b/drivers/clk/sunxi-ng/ccu_common.h
> > @@ -0,0 +1,74 @@
> > +/*
> > + * Copyright (c) 2016 Maxime Ripard. All rights reserved.
> > + *
> > + * This software is licensed under the terms of the GNU General Public
> > + * License version 2, as published by the Free Software Foundation, and
> > + * may be copied, distributed, and modified under those terms.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#ifndef _COMMON_H_
> > +#define _COMMON_H_
> > +
> > +#include <linux/compiler.h>
> > +#include <linux/clk-provider.h>
> > +
> > +#define CCU_FEATURE_GATE               BIT(0)
> > +#define CCU_FEATURE_LOCK               BIT(1)
> 
> *_PLL_LOCK would be clearer that this implements a PLL lock indicator.
> Or maybe a comment.

I'll change it for PLL_LOCK

> 
> > +#define CCU_FEATURE_FRACTIONAL         BIT(2)
> > +#define CCU_FEATURE_VARIABLE_PREDIV    BIT(3)
> > +#define CCU_FEATURE_FIXED_PREDIV       BIT(4)
> > +#define CCU_FEATURE_FIXED_POSTDIV      BIT(5)
> > +
> > +struct device_node;
> > +
> > +#define SUNXI_HW_INIT(_name, _parent, _ops, _flags)                    \
> > +       &(struct clk_init_data) {                                       \
> > +               .flags          = _flags,                               \
> > +               .name           = _name,                                \
> > +               .parent_names   = (const char *[]) { _parent },         \
> > +               .num_parents    = 1,                                    \
> > +               .ops            = _ops,                                 \
> > +       }
> > +
> > +#define SUNXI_HW_INIT_PARENTS(_name, _parents, _ops, _flags)           \
> > +       &(struct clk_init_data) {                                       \
> > +               .flags          = _flags,                               \
> > +               .name           = _name,                                \
> > +               .parent_names   = _parents,                             \
> > +               .num_parents    = ARRAY_SIZE(_parents),                 \
> > +               .ops            = _ops,                                 \
> > +       }
> > +
> > +struct ccu_common {
> > +       void __iomem    *base;
> > +       unsigned long   reg;
> 
> This seems quite large, considering the address space of the CCU,
> and you using u16 or u32 for the same thing on the reset control side.

Indeed, what about u16?

> 
> > +
> > +       unsigned long   features;
> > +       spinlock_t      *lock;
> > +       struct clk_hw   hw;
> > +};
> > +
> > +static inline struct ccu_common *hw_to_ccu_common(struct clk_hw *hw)
> > +{
> > +       return container_of(hw, struct ccu_common, hw);
> > +}
> > +
> > +struct sunxi_ccu_desc {
> > +       struct ccu_common       **clks;
> > +       unsigned long           num_clks;
> > +
> > +       struct ccu_reset_map    *resets;
> > +       unsigned long           num_resets;
> > +};
> > +
> > +void ccu_helper_wait_for_lock(struct ccu_common *common, u32 lock);
> > +
> > +int sunxi_ccu_probe(struct device_node *node,
> > +                   const struct sunxi_ccu_desc *desc);
> > +
> > +#endif /* _COMMON_H_ */
> > diff --git a/drivers/clk/sunxi-ng/ccu_factor.h b/drivers/clk/sunxi-ng/ccu_factor.h
> > new file mode 100644
> > index 000000000000..e7cc564aaea0
> > --- /dev/null
> > +++ b/drivers/clk/sunxi-ng/ccu_factor.h
> > @@ -0,0 +1,15 @@
> > +#ifndef _CLK_FACTOR_H_
> > +#define _CLK_FACTOR_H_
> > +
> > +struct ccu_factor {
> > +       u8      shift;
> > +       u8      width;
> > +};
> > +
> > +#define SUNXI_CLK_FACTOR(_shift, _width)       \
> > +       {                                       \
> > +               .shift  = _shift,               \
> > +               .width  = _width,               \
> > +       }
> > +
> > +#endif /* _CLK_FACTOR_H_ */
> > diff --git a/drivers/clk/sunxi-ng/ccu_mux.h b/drivers/clk/sunxi-ng/ccu_mux.h
> > new file mode 100644
> > index 000000000000..17cedad4e433
> > --- /dev/null
> > +++ b/drivers/clk/sunxi-ng/ccu_mux.h
> 
> As far as I can tell there are no users of this file within this patch or the
> following patches before the mux clock support one. It'd be easier to understand
> if this part was moved to the mux clock patch.

Will do.

> 
> > @@ -0,0 +1,20 @@
> > +#ifndef _CCU_MUX_H_
> > +#define _CCU_MUX_H_
> > +
> > +#include "common.h"
> > +
> > +struct ccu_mux_internal {
> > +       u8      shift;
> > +       u8      width;
> > +
> > +       u8      *map;
> 
> I assume map is a table?
> 
> > +};
> > +
> > +#define SUNXI_CLK_MUX(_shift, _width, _map)    \
> > +       {                                       \
> > +               .map    = _map,                 \
> > +               .shift  = _shift,               \
> > +               .width  = _width,               \
> > +       }
> > +
> > +#endif /* _CCU_MUX_H_ */
> > diff --git a/drivers/clk/sunxi-ng/ccu_reset.c b/drivers/clk/sunxi-ng/ccu_reset.c
> > new file mode 100644
> > index 000000000000..6c31d48783a7
> > --- /dev/null
> > +++ b/drivers/clk/sunxi-ng/ccu_reset.c
> > @@ -0,0 +1,55 @@
> > +/*
> > + * Copyright (C) 2016 Maxime Ripard
> > + * Maxime Ripard <maxime.ripard@free-electrons.com>
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License as
> > + * published by the Free Software Foundation; either version 2 of
> > + * the License, or (at your option) any later version.
> > + */
> > +
> > +#include <linux/io.h>
> > +#include <linux/reset-controller.h>
> > +
> > +#include "ccu_reset.h"
> > +
> > +static int ccu_reset_assert(struct reset_controller_dev *rcdev,
> > +                           unsigned long id)
> > +{
> > +       struct ccu_reset *ccu = rcdev_to_ccu_reset(rcdev);
> > +       const struct ccu_reset_map *map = &ccu->reset_map[id];
> > +       unsigned long flags;
> > +       u32 reg;
> > +
> > +       spin_lock_irqsave(ccu->lock, flags);
> > +
> > +       reg = readl(ccu->base + map->reg);
> > +       writel(reg & ~map->bit, ccu->base + map->reg);
> > +
> > +       spin_unlock_irqrestore(ccu->lock, flags);
> > +
> > +       return 0;
> > +}
> > +
> > +static int ccu_reset_deassert(struct reset_controller_dev *rcdev,
> > +                             unsigned long id)
> > +{
> > +       struct ccu_reset *ccu = rcdev_to_ccu_reset(rcdev);
> > +       const struct ccu_reset_map *map = &ccu->reset_map[id];
> > +       unsigned long flags;
> > +       u32 reg;
> > +
> > +       spin_lock_irqsave(ccu->lock, flags);
> > +
> > +       reg = readl(ccu->base + map->reg);
> > +       writel(reg | map->bit, ccu->base + map->reg);
> > +
> > +       spin_unlock_irqrestore(ccu->lock, flags);
> > +
> > +       return 0;
> > +}
> > +
> > +const struct reset_control_ops ccu_reset_ops = {
> > +       .assert         = ccu_reset_assert,
> > +       .deassert       = ccu_reset_deassert,
> > +};
> > diff --git a/drivers/clk/sunxi-ng/ccu_reset.h b/drivers/clk/sunxi-ng/ccu_reset.h
> > new file mode 100644
> > index 000000000000..36a4679210bd
> > --- /dev/null
> > +++ b/drivers/clk/sunxi-ng/ccu_reset.h
> > @@ -0,0 +1,40 @@
> > +/*
> > + * Copyright (c) 2016 Maxime Ripard. All rights reserved.
> > + *
> > + * This software is licensed under the terms of the GNU General Public
> > + * License version 2, as published by the Free Software Foundation, and
> > + * may be copied, distributed, and modified under those terms.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#ifndef _CCU_RESET_H_
> > +#define _CCU_RESET_H_
> > +
> > +#include <linux/reset-controller.h>
> > +
> > +struct ccu_reset_map {
> > +       u16     reg;
> > +       u32     bit;
> > +};
> > +
> > +
> > +struct ccu_reset {
> > +       void __iomem                    *base;
> > +       struct ccu_reset_map            *reset_map;
> > +       spinlock_t                      *lock;
> > +
> > +       struct reset_controller_dev     rcdev;
> > +};
> > +
> > +static inline struct ccu_reset *rcdev_to_ccu_reset(struct reset_controller_dev *rcdev)
> > +{
> > +       return container_of(rcdev, struct ccu_reset, rcdev);
> > +}
> > +
> > +extern const struct reset_control_ops ccu_reset_ops;
> > +
> > +#endif /* _CCU_RESET_H_ */
> 
> The reset control code looks good.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160515/f96ad50b/attachment.sig>

  reply	other threads:[~2016-05-15 18:31 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-08 20:01 [PATCH 00/16] clk: sunxi: introduce "modern" clock support Maxime Ripard
2016-05-08 20:01 ` [PATCH 01/16] clk: fix critical clock locking Maxime Ripard
2016-05-09 22:11   ` Stephen Boyd
2016-05-13  7:50     ` Maxime Ripard
2016-05-08 20:01 ` [PATCH 02/16] clk: sunxi-ng: Add common infrastructure Maxime Ripard
2016-05-09 10:01   ` Chen-Yu Tsai
2016-05-15 18:31     ` Maxime Ripard [this message]
2016-05-16  7:02       ` Chen-Yu Tsai
2016-05-16  8:02       ` Jean-Francois Moine
2016-05-16 20:15         ` Maxime Ripard
2016-05-17  6:54           ` Jean-Francois Moine
2016-05-18 19:59             ` Maxime Ripard
2016-05-08 20:01 ` [PATCH 03/16] clk: sunxi-ng: Add fixed factor clock support Maxime Ripard
2016-05-09 10:05   ` Chen-Yu Tsai
2016-05-16 13:15     ` Jean-Francois Moine
2016-05-16 21:08     ` Maxime Ripard
2016-05-08 20:01 ` [PATCH 04/16] clk: sunxi-ng: Add gate " Maxime Ripard
2016-05-08 20:01 ` [PATCH 05/16] clk: sunxi-ng: Add mux " Maxime Ripard
2016-05-21 16:18   ` Chen-Yu Tsai
2016-05-22 19:20     ` Maxime Ripard
2016-05-08 20:01 ` [PATCH 06/16] clk: sunxi-ng: Add divider table clock Maxime Ripard
2016-05-21 16:30   ` Chen-Yu Tsai
2016-05-08 20:01 ` [PATCH 07/16] clk: sunxi-ng: Add phase clock support Maxime Ripard
2016-05-21 16:43   ` Chen-Yu Tsai
2016-05-23 17:01     ` Maxime Ripard
2016-05-24  9:01       ` Chen-Yu Tsai
2016-05-08 20:01 ` [PATCH 08/16] clk: sunxi-ng: Add M-factor " Maxime Ripard
2016-05-11  6:46   ` Jean-Francois Moine
2016-05-15 18:51     ` Maxime Ripard
2016-05-21 17:09   ` Chen-Yu Tsai
2016-05-22 19:22     ` Maxime Ripard
2016-05-08 20:01 ` [PATCH 09/16] clk: sunxi-ng: Add P-factor " Maxime Ripard
2016-05-08 20:01 ` [PATCH 10/16] clk: sunxi-ng: Add M-P factor " Maxime Ripard
2016-05-23 13:45   ` Chen-Yu Tsai
2016-05-23 17:18     ` Maxime Ripard
2016-05-24  4:14       ` Chen-Yu Tsai
2016-05-24 21:07         ` Maxime Ripard
2016-05-08 20:01 ` [PATCH 11/16] clk: sunxi-ng: Add N-K-factor " Maxime Ripard
2016-05-23 13:58   ` Chen-Yu Tsai
2016-05-08 20:01 ` [PATCH 12/16] clk: sunxi-ng: Add N-M-factor " Maxime Ripard
2016-05-09  7:24   ` Jean-Francois Moine
2016-05-15 19:04     ` Maxime Ripard
2016-05-08 20:01 ` [PATCH 13/16] clk: sunxi-ng: Add N-K-M Factor clock Maxime Ripard
2016-05-11  8:45   ` Jean-Francois Moine
2016-05-15 19:08     ` Maxime Ripard
2016-05-23 14:10   ` Chen-Yu Tsai
2016-05-08 20:01 ` [PATCH 14/16] clk: sunxi-ng: Add N-K-M-P factor clock Maxime Ripard
2016-05-11  8:49   ` Jean-Francois Moine
2016-05-23 14:36   ` Chen-Yu Tsai
2016-05-30  7:57     ` Maxime Ripard
2016-05-08 20:01 ` [PATCH 15/16] clk: sunxi-ng: Add H3 clocks Maxime Ripard
2016-05-09  7:39   ` Jean-Francois Moine
2016-05-15 19:18     ` Maxime Ripard
2016-05-13  9:45   ` Jean-Francois Moine
2016-05-18 14:02     ` Maxime Ripard
2016-05-18 16:23       ` Jean-Francois Moine
2016-05-18 16:27       ` Jean-Francois Moine
2016-05-16 13:47   ` Jean-Francois Moine
2016-05-18 21:20     ` Maxime Ripard
2016-05-30 16:15   ` Chen-Yu Tsai
2016-06-01 19:19     ` Maxime Ripard
2016-06-03  6:42       ` Chen-Yu Tsai
2016-06-03  6:55         ` Chen-Yu Tsai
2016-05-08 20:01 ` [PATCH 16/16] ARM: dt: sun8i: switch the H3 to the new CCU driver Maxime Ripard

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160515183122.GA27618@lukather \
    --to=maxime.ripard@free-electrons.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).