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:   Thu, 22 Jun 2017 18:12:09 +0800
From:   Chunyan Zhang <zhang.lyra@...il.com>
To:     Stephen Boyd <sboyd@...eaurora.org>
Cc:     Chunyan Zhang <chunyan.zhang@...eadtrum.com>,
        Michael Turquette <mturquette@...libre.com>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>, linux-clk@...r.kernel.org,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        Arnd Bergmann <arnd@...db.de>, Mark Brown <broonie@...nel.org>,
        Xiaolong Zhang <xiaolong.zhang@...eadtrum.com>,
        Orson Zhai <orson.zhai@...eadtrum.com>,
        Geng Ren <geng.ren@...eadtrum.com>,
        Ben Li <ben.li@...eadtrum.com>
Subject: Re: [PATCH V1 2/9] clk: sprd: Add common infrastructure

On 20 June 2017 at 09:29, Stephen Boyd <sboyd@...eaurora.org> wrote:
> On 06/18, Chunyan Zhang wrote:
>> Added Spreadtrum's clock driver common structure and registration code.
>>
>> Signed-off-by: Chunyan Zhang <chunyan.zhang@...eadtrum.com>
>> ---
>>  drivers/clk/Makefile          |  1 +
>>  drivers/clk/sprd/Makefile     |  3 ++
>>  drivers/clk/sprd/ccu_common.c | 78 +++++++++++++++++++++++++++++++++++++
>>  drivers/clk/sprd/ccu_common.h | 90 +++++++++++++++++++++++++++++++++++++++++++
>>  4 files changed, 172 insertions(+)
>>  create mode 100644 drivers/clk/sprd/Makefile
>>  create mode 100644 drivers/clk/sprd/ccu_common.c
>>  create mode 100644 drivers/clk/sprd/ccu_common.h
>>
>> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
>> index c19983a..1d62721 100644
>> --- a/drivers/clk/Makefile
>> +++ b/drivers/clk/Makefile
>> @@ -81,6 +81,7 @@ obj-$(CONFIG_COMMON_CLK_SAMSUNG)    += samsung/
>>  obj-$(CONFIG_ARCH_SIRF)                      += sirf/
>>  obj-$(CONFIG_ARCH_SOCFPGA)           += socfpga/
>>  obj-$(CONFIG_PLAT_SPEAR)             += spear/
>> +obj-$(CONFIG_ARCH_SPRD)                      += sprd/
>>  obj-$(CONFIG_ARCH_STI)                       += st/
>>  obj-$(CONFIG_ARCH_SUNXI)             += sunxi/
>>  obj-$(CONFIG_ARCH_SUNXI)             += sunxi-ng/
>> diff --git a/drivers/clk/sprd/Makefile b/drivers/clk/sprd/Makefile
>> new file mode 100644
>> index 0000000..8f802b2
>> --- /dev/null
>> +++ b/drivers/clk/sprd/Makefile
>> @@ -0,0 +1,3 @@
>> +ifneq ($(CONFIG_OF),)
>> +obj-y        += ccu_common.o
>> +endif
>
> I'd prefer a Kconfig for SPRD clk drivers instead of this
> CONFIG_OF check. Then we can compile test the sprd code in
> configurations that don't have CONFIG_ARCH_SPRD set too.

Ok, make sense, will address this in the next version.

>
>> diff --git a/drivers/clk/sprd/ccu_common.c b/drivers/clk/sprd/ccu_common.c
>> new file mode 100644
>> index 0000000..911f4ba
>> --- /dev/null
>> +++ b/drivers/clk/sprd/ccu_common.c
>> @@ -0,0 +1,78 @@
>> +/*
>> + * Spreadtrum clock infrastructure
>> + *
>> + * Copyright (C) 2017 Spreadtrum, Inc.
>> + *
>> + * SPDX-License-Identifier: GPL-2.0
>> + */
>> +
>> +#include "ccu_common.h"
>> +
>> +static inline void __iomem *ccu_find_base(struct ccu_addr_map *maps,
>> +                                       unsigned int num, unsigned int reg)
>> +{
>> +     int i;
>> +
>> +     for (i = 0; i < num; i++)
>> +             if ((reg & 0xffff0000) == maps[i].phy)
>
> What is this?

You can look at ccu-sc9860.c, different from sunxi-ng, we specify the
whole register address rather than register offset when initializing
Spreadtrum clocks, the high 16 bits is the base address which is
configured in DT, and is mapped in the board clock files like
ccu-sc9860.c, I will write more in commit message to explain this when
cooking next version of the patchset.

>
>> +                     return maps[i].virt;
>> +
>> +     return 0;
>> +}
>> +
>> +int sprd_ccu_probe(struct device_node *node, struct ccu_addr_map *maps,
>> +                unsigned int count, const struct sprd_ccu_desc *desc)
>> +{
>> +     int i, ret = 0;
>> +     struct ccu_common *cclk;
>> +     struct clk_hw *hw;
>> +
>> +     for (i = 0; i < desc->num_ccu_clks; i++) {
>> +             cclk = desc->ccu_clks[i];
>> +             if (!cclk)
>> +                     continue;
>> +
>> +             cclk->base = ccu_find_base(maps, count, cclk->reg);
>> +             if (!cclk->base) {
>> +                     pr_err("%s: No mapped address found for clock(0x%x)\n",
>> +                             __func__, cclk->reg);
>> +                     return -EINVAL;
>> +             }
>> +             cclk->reg = cclk->reg & 0xffff;
>> +     }
>> +
>> +     for (i = 0; i < desc->hw_clks->num; i++) {
>> +
>> +             hw = desc->hw_clks->hws[i];
>> +
>> +             if (!hw)
>> +                     continue;
>> +
>> +             ret = clk_hw_register(NULL, hw);
>> +             if (ret) {
>> +                     pr_err("Couldn't register clock %d - %s\n",
>> +                            i, hw->init->name);
>> +                     goto err_clk_unreg;
>> +             }
>> +     }
>> +
>> +     ret = of_clk_add_hw_provider(node, of_clk_hw_onecell_get,
>> +                                  desc->hw_clks);
>> +     if (ret) {
>> +             pr_err("Failed to add clock provider.\n");
>> +             goto err_clk_unreg;
>> +     }
>> +
>> +     return 0;
>> +
>> +err_clk_unreg:
>> +     while (--i >= 0) {
>> +             hw = desc->hw_clks->hws[i];
>> +             if (!hw)
>> +                     continue;
>> +
>> +             clk_hw_unregister(hw);
>> +     }
>> +
>> +     return ret;
>> +}
>> diff --git a/drivers/clk/sprd/ccu_common.h b/drivers/clk/sprd/ccu_common.h
>> new file mode 100644
>> index 0000000..ff07772
>> --- /dev/null
>> +++ b/drivers/clk/sprd/ccu_common.h
>> @@ -0,0 +1,90 @@
>> +/*
>> + * Spreadtrum clock infrastructure
>> + *
>> + * Copyright (C) 2017 Spreadtrum, Inc.
>> + *
>> + * SPDX-License-Identifier: GPL-2.0
>> + */
>> +
>> +#ifndef _CCU_COMMON_H_
>> +#define _CCU_COMMON_H_
>> +
>> +#include <linux/clk-provider.h>
>> +
>> +struct device_node;
>> +
>> +#define CLK_HW_INIT_NO_PARENT(_name, _ops, _flags)   \
>> +     (&(struct clk_init_data) {                      \
>> +             .flags          = _flags,               \
>> +             .name           = _name,                \
>> +             .parent_names   = NULL,                 \
>> +             .num_parents    = 0,                    \
>> +             .ops            = _ops,                 \
>> +     })
>> +
>> +#define CLK_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 CLK_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,                         \
>> +     })
>> +
>> +#define CLK_FIXED_FACTOR(_struct, _name, _parent,                    \
>> +                     _div, _mult, _flags)                            \
>> +     struct clk_fixed_factor _struct = {                             \
>> +             .div            = _div,                                 \
>> +             .mult           = _mult,                                \
>> +             .hw.init        = CLK_HW_INIT(_name,                    \
>> +                                           _parent,                  \
>> +                                           &clk_fixed_factor_ops,    \
>> +                                           _flags),                  \
>> +     }
>> +
>> +struct ccu_common {
>> +     void __iomem    *base;
>> +     u32             reg;
>> +     spinlock_t      *lock;
>> +     struct clk_hw   hw;
>> +};
>> +
>> +struct ccu_addr_map {
>> +     phys_addr_t phy;
>> +     void __iomem *virt;
>> +};
>> +
>> +static inline u32 ccu_readl(struct ccu_common *common)
>> +{
>> +     return readl(common->base + common->reg);
>> +}
>> +
>> +static inline void ccu_writel(u32 val, struct ccu_common *common)
>> +{
>> +     writel(val, common->base + common->reg);
>> +}
>> +
>> +static inline struct ccu_common *hw_to_ccu_common(struct clk_hw *hw)
>> +{
>> +     return container_of(hw, struct ccu_common, hw);
>> +}
>> +
>> +struct sprd_ccu_desc {
>> +     struct ccu_common               **ccu_clks;
>> +     unsigned long                   num_ccu_clks;
>> +     struct clk_hw_onecell_data      *hw_clks;
>> +};
>> +
>> +int sprd_ccu_probe(struct device_node *node, struct ccu_addr_map *maps,
>> +                unsigned int count, const struct sprd_ccu_desc *desc);
>> +
>> +#endif /* _CCU_COMMON_H_ */
>
> Do you call them CCUs internally? I thought CCU was a sunxi
> thing, so it may make more sense to call it whatever you call the
> clock controller on your hardware.

OK, will address that.

Thanks,
Chunyan

>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ