* Re: [PATCH v3 3/8] mailbox: Add transmit done by blocking option
From: Mikko Perttunen @ 2018-08-08 11:38 UTC (permalink / raw)
To: Jassi Brar, Mikko Perttunen
Cc: Greg KH, Thierry Reding, Jon Hunter, Devicetree List,
linux-serial, linux-tegra, ", linux-arm-kernel",
linux-mediatek, srv_heupstream, Linux Kernel Mailing List
In-Reply-To: <907bac36-0c54-8f38-0f5e-f59196d414c6@kapsi.fi>
On 04.08.2018 13:45, Mikko Perttunen wrote:
> On 08/03/2018 03:54 PM, Jassi Brar wrote:
>> On Mon, Jul 2, 2018 at 5:10 PM, Mikko Perttunen
>> <mperttunen@nvidia.com> wrote:
>>> Add a new TXDONE option, TXDONE_BY_BLOCK. With this option, the
>>> send_data function of the mailbox driver is expected to block until
>>> the message has been sent. The new option is used with the Tegra
>>> Combined UART driver to minimize unnecessary overhead when transmitting
>>> data.
>>>
>> 1) TXDONE_BY_BLOCK flag :-
>> Have you tried setting the flag
>> mbox_chan->mbox_client->tx_block ?
>
> No - I suppose I should have done that. I'm a bit concerned about
> overhead as send_data may be called thousands of times per second, so I
> tried to make it as close as possible to the downstream driver that just
> pokes the mailbox register directly.
I tried using polling in the mailbox framework. Some printing is done
from atomic context so it seems tx_block cannot be used -
wait_for_completion_timeout understandably does not work in atomic
context. I also tried without tx_block, in which case I got some
horribly garbled output, but "Try increasing MBOX_TX_QUEUE_LEN" was
readable there.
Any opinions?
Thanks,
Mikko
>
>>
>> 2) Implementing TEGRA_HSP_MBOX_TYPE_SM :-
>> In mailbox framework, a controller is a collection of identical
>> channels. That is, instances of the same class.
>> So ideally, in probe you should populate a controller for each
>> type of channel, i.e, DB, SM, SS and AS.
>
> Hmm, yes, I guess this would be possible if I change the mailbox core to
> allow registering multiple controllers per device.
>
> Thanks!
> Mikko
>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 02/18] clk: intel: Add clock driver for Intel MIPS SoCs
From: yixin zhu @ 2018-08-08 8:52 UTC (permalink / raw)
To: Stephen Boyd, Songjun Wu, chuanhua.lei, hua.ma, qi-ming.wu
Cc: linux-mips, linux-clk, linux-serial, devicetree,
Michael Turquette, linux-kernel, Rob Herring, Mark Rutland
In-Reply-To: <153370742214.220756.2039365625963765922@swboyd.mtv.corp.google.com>
On 8/8/2018 1:50 PM, Stephen Boyd wrote:
> Quoting Songjun Wu (2018-08-02 20:02:21)
>> From: Yixin Zhu <yixin.zhu@linux.intel.com>
>>
>> This driver provides PLL clock registration as well as various clock
>> branches, e.g. MUX clock, gate clock, divider clock and so on.
>>
>> PLLs that provide clock to DDR, CPU and peripherals are shown below:
>>
>> +---------+
>> |--->| LCPLL3 0|--PCIe clk-->
>> XO | +---------+
>> +-----------|
>> | +---------+
>> | | 3|--PAE clk-->
>> |--->| PLL0B 2|--GSWIP clk-->
>> | | 1|--DDR clk-->DDR PHY clk-->
>> | | 0|--CPU1 clk--+ +-----+
>> | +---------+ |--->0 |
>> | | MUX |--CPU clk-->
>> | +---------+ |--->1 |
>> | | 0|--CPU0 clk--+ +-----+
>> |--->| PLLOA 1|--SSX4 clk-->
>> | 2|--NGI clk-->
>> | 3|--CBM clk-->
>> +---------+
> Thanks for the picture!
Thanks for the review.
>
>> Signed-off-by: Yixin Zhu <yixin.zhu@linux.intel.com>
>> Signed-off-by: Songjun Wu <songjun.wu@linux.intel.com>
>> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
>> index 0bb25dd009d1..d929ca4607cf 100644
>> --- a/drivers/clk/Makefile
>> +++ b/drivers/clk/Makefile
>> @@ -72,6 +72,9 @@ obj-$(CONFIG_ARCH_HISI) += hisilicon/
>> obj-y += imgtec/
>> obj-$(CONFIG_ARCH_MXC) += imx/
>> obj-$(CONFIG_MACH_INGENIC) += ingenic/
>> +ifeq ($(CONFIG_COMMON_CLK), y)
>> +obj-y +=intel/
>> +endif
> Why not obj-$(CONFIG_INTEL_CCF) or something like that?
Will use obj-$(CONFIG_INTEL_CCF)
>> obj-$(CONFIG_ARCH_KEYSTONE) += keystone/
>> obj-$(CONFIG_MACH_LOONGSON32) += loongson1/
>> obj-y += mediatek/
>> diff --git a/drivers/clk/intel/Kconfig b/drivers/clk/intel/Kconfig
>> new file mode 100644
>> index 000000000000..c7d3fb1721fa
>> --- /dev/null
>> +++ b/drivers/clk/intel/Kconfig
>> @@ -0,0 +1,20 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +config INTEL_CGU_CLK
>> + depends on COMMON_CLK
>> + depends on INTEL_MIPS || COMPILE_TEST
>> + select MFD_SYSCON
>> + bool "Intel clock controller support"
>> + help
>> + This driver support Intel CGU (Clock Generation Unit).
> Is it really called a clock generation unit? Or that's just copied from
> sunxi driver?
Yes, It's called clock generation unit(CGU) in our HW chip spec.
>> +
>> +choice
>> + prompt "SoC platform selection"
>> + depends on INTEL_CGU_CLK
>> + default INTEL_GRX500_CGU_CLK
>> +
>> +config INTEL_GRX500_CGU_CLK
>> + bool "GRX500 CLK"
>> + help
>> + Clock driver of GRX500 platform.
>> +
>> +endchoice
>> diff --git a/drivers/clk/intel/Makefile b/drivers/clk/intel/Makefile
>> new file mode 100644
>> index 000000000000..16a0138e52c2
>> --- /dev/null
>> +++ b/drivers/clk/intel/Makefile
>> @@ -0,0 +1,7 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Makefile for intel specific clk
>> +
>> +obj-$(CONFIG_INTEL_CGU_CLK) += clk-cgu.o clk-cgu-pll.o
>> +ifneq ($(CONFIG_INTEL_GRX500_CGU_CLK),)
>> + obj-y += clk-grx500.o
>> +endif
>> diff --git a/drivers/clk/intel/clk-cgu-pll.c b/drivers/clk/intel/clk-cgu-pll.c
>> new file mode 100644
>> index 000000000000..20759bc27e95
>> --- /dev/null
>> +++ b/drivers/clk/intel/clk-cgu-pll.c
>> @@ -0,0 +1,166 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2018 Intel Corporation.
>> + * Zhu YiXin <Yixin.zhu@intel.com>
>> + */
>> +
>> +#include <linux/clk.h>
> Is this include used?
Not used. Will remove it.
>> +#include <linux/clk-provider.h>
>> +#include <linux/clkdev.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/of.h>
>> +#include <linux/of_address.h>
>> +#include <linux/regmap.h>
>> +#include <linux/slab.h>
>> +
>> +#include "clk-cgu-pll.h"
>> +#include "clk-cgu.h"
>> +
>> +#define to_intel_clk_pll(_hw) container_of(_hw, struct intel_clk_pll, hw)
>> +
>> +/*
>> + * Calculate formula:
>> + * rate = (prate * mult + (prate * frac) / frac_div) / div
>> + */
>> +static unsigned long
>> +intel_pll_calc_rate(unsigned long prate, unsigned int mult,
>> + unsigned int div, unsigned int frac,
>> + unsigned int frac_div)
>> +{
>> + u64 crate, frate, rate64;
>> +
>> + rate64 = prate;
>> + crate = rate64 * mult;
>> +
>> + if (frac) {
>> + frate = rate64 * frac;
>> + do_div(frate, frac_div);
>> + crate += frate;
>> + }
>> + do_div(crate, div);
>> +
>> + return (unsigned long)crate;
>> +}
>> +
>> +static void
>> +grx500_pll_get_params(struct intel_clk_pll *pll, unsigned int *mult,
>> + unsigned int *frac)
>> +{
>> + *mult = intel_get_clk_val(pll->map, pll->reg, 2, 7);
>> + *frac = intel_get_clk_val(pll->map, pll->reg, 9, 21);
>> +}
>> +
>> +static int intel_wait_pll_lock(struct intel_clk_pll *pll, int bit_idx)
>> +{
>> + unsigned int val;
>> +
>> + return regmap_read_poll_timeout(pll->map, pll->reg, val,
>> + val & BIT(bit_idx), 10, 1000);
>> +}
>> +
>> +static unsigned long
>> +intel_grx500_pll_recalc_rate(struct clk_hw *hw, unsigned long prate)
>> +{
>> + struct intel_clk_pll *pll = to_intel_clk_pll(hw);
>> + unsigned int mult, frac;
>> +
>> + grx500_pll_get_params(pll, &mult, &frac);
>> +
>> + return intel_pll_calc_rate(prate, mult, 1, frac, BIT(20));
>> +}
>> +
>> +static int intel_grx500_pll_is_enabled(struct clk_hw *hw)
>> +{
>> + struct intel_clk_pll *pll = to_intel_clk_pll(hw);
>> +
>> + if (intel_wait_pll_lock(pll, 1)) {
>> + pr_err("%s: pll: %s is not locked!\n",
>> + __func__, clk_hw_get_name(hw));
>> + return 0;
>> + }
>> +
>> + return intel_get_clk_val(pll->map, pll->reg, 1, 1);
>> +}
>> +
>> +const static struct clk_ops intel_grx500_pll_ops = {
> Should be static const struct ...
Will update it.
>> + .recalc_rate = intel_grx500_pll_recalc_rate,
>> + .is_enabled = intel_grx500_pll_is_enabled,
>> +};
>> +
>> +static struct clk
>> +*intel_clk_register_pll(struct intel_clk_provider *ctx,
>> + enum intel_pll_type type, const char *cname,
>> + const char *const *pname, u8 num_parents,
>> + unsigned long flags, unsigned int reg,
>> + const struct intel_pll_rate_table *table,
>> + unsigned int mult, unsigned int div, unsigned int frac)
>> +{
>> + struct clk_init_data init;
>> + struct intel_clk_pll *pll;
>> + struct clk_hw *hw;
>> + int ret, i;
>> +
>> + if (type != pll_grx500) {
>> + pr_err("%s: pll type %d not supported!\n",
>> + __func__, type);
>> + return ERR_PTR(-EINVAL);
>> + }
>> + init.name = cname;
>> + init.ops = &intel_grx500_pll_ops;
>> + init.flags = CLK_IS_BASIC;
> Don't use this flag unless you have some reason to need it.
Will remove it.
>> + init.parent_names = pname;
>> + init.num_parents = num_parents;
>> +
>> + pll = kzalloc(sizeof(*pll), GFP_KERNEL);
>> + if (!pll)
>> + return ERR_PTR(-ENOMEM);
>> + pll->map = ctx->map;
>> + pll->reg = reg;
>> + pll->flags = flags;
>> + pll->mult = mult;
>> + pll->div = div;
>> + pll->frac = frac;
>> + pll->hw.init = &init;
>> + if (table) {
>> + for (i = 0; table[i].rate != 0; i++)
>> + ;
>> + pll->table_sz = i;
>> + pll->rate_table = kmemdup(table, i * sizeof(table[0]),
>> + GFP_KERNEL);
>> + if (!pll->rate_table) {
>> + ret = -ENOMEM;
>> + goto err_free_pll;
>> + }
>> + }
>> + hw = &pll->hw;
>> + ret = clk_hw_register(NULL, hw);
>> + if (ret)
>> + goto err_free_pll;
>> +
>> + return hw->clk;
>> +
>> +err_free_pll:
>> + kfree(pll);
>> + return ERR_PTR(ret);
>> +}
>> +
>> +void intel_clk_register_plls(struct intel_clk_provider *ctx,
>> + struct intel_pll_clk *list, unsigned int nr_clk)
>> +{
>> + struct clk *clk;
>> + int i;
>> +
>> + for (i = 0; i < nr_clk; i++, list++) {
>> + clk = intel_clk_register_pll(ctx, list->type, list->name,
>> + list->parent_names, list->num_parents,
>> + list->flags, list->reg, list->rate_table,
>> + list->mult, list->div, list->frac);
>> + if (IS_ERR(clk)) {
>> + pr_err("%s: failed to register pll: %s\n",
>> + __func__, list->name);
>> + continue;
>> + }
>> +
>> + intel_clk_add_lookup(ctx, clk, list->id);
>> + }
>> +}
>> diff --git a/drivers/clk/intel/clk-cgu-pll.h b/drivers/clk/intel/clk-cgu-pll.h
>> new file mode 100644
>> index 000000000000..3e7cff1d5e16
>> --- /dev/null
>> +++ b/drivers/clk/intel/clk-cgu-pll.h
>> @@ -0,0 +1,34 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Copyright(c) 2018 Intel Corporation.
>> + * Zhu YiXin <Yixin.zhu@intel.com>
>> + */
>> +
>> +#ifndef __INTEL_CLK_PLL_H
>> +#define __INTEL_CLK_PLL_H
>> +
>> +enum intel_pll_type {
>> + pll_grx500,
>> +};
>> +
>> +struct intel_pll_rate_table {
>> + unsigned long prate;
>> + unsigned long rate;
>> + unsigned int mult;
>> + unsigned int div;
>> + unsigned int frac;
>> +};
>> +
>> +struct intel_clk_pll {
>> + struct clk_hw hw;
>> + struct regmap *map;
>> + unsigned int reg;
>> + unsigned long flags;
>> + unsigned int mult;
>> + unsigned int div;
>> + unsigned int frac;
>> + unsigned int table_sz;
>> + const struct intel_pll_rate_table *rate_table;
>> +};
>> +
>> +#endif /* __INTEL_CLK_PLL_H */
>> diff --git a/drivers/clk/intel/clk-cgu.c b/drivers/clk/intel/clk-cgu.c
>> new file mode 100644
>> index 000000000000..10cacbe0fbcd
>> --- /dev/null
>> +++ b/drivers/clk/intel/clk-cgu.c
>> @@ -0,0 +1,470 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2018 Intel Corporation.
>> + * Zhu YiXin <Yixin.zhu@intel.com>
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/clk-provider.h>
>> +#include <linux/clkdev.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/of.h>
>> +#include <linux/of_address.h>
>> +#include <linux/regmap.h>
>> +#include <linux/slab.h>
>> +
>> +#include "clk-cgu-pll.h"
>> +#include "clk-cgu.h"
>> +
>> +#define GATE_HW_REG_STAT(reg) (reg)
>> +#define GATE_HW_REG_EN(reg) ((reg) + 0x4)
>> +#define GATE_HW_REG_DIS(reg) ((reg) + 0x8)
>> +
>> +#define to_intel_clk_mux(_hw) container_of(_hw, struct intel_clk_mux, hw)
>> +#define to_intel_clk_divider(_hw) \
>> + container_of(_hw, struct intel_clk_divider, hw)
>> +#define to_intel_clk_gate(_hw) container_of(_hw, struct intel_clk_gate, hw)
>> +
>> +void intel_set_clk_val(struct regmap *map, u32 reg, u8 shift,
>> + u8 width, u32 set_val)
>> +{
>> + u32 mask = GENMASK(width + shift, shift);
>> +
>> + regmap_update_bits(map, reg, mask, set_val << shift);
>> +}
>> +
>> +u32 intel_get_clk_val(struct regmap *map, u32 reg, u8 shift,
>> + u8 width)
>> +{
>> + u32 val;
>> +
>> + if (regmap_read(map, reg, &val)) {
>> + WARN_ONCE(1, "Failed to read clk reg: 0x%x\n", reg);
>> + return 0;
>> + }
>> + val >>= shift;
>> + val &= BIT(width) - 1;
>> +
>> + return val;
>> +}
>> +
>> +void intel_clk_add_lookup(struct intel_clk_provider *ctx,
>> + struct clk *clk, unsigned int id)
>> +{
>> + pr_debug("Add clk: %s, id: %u\n", __clk_get_name(clk), id);
>> + if (ctx->clk_data.clks && id)
>> + ctx->clk_data.clks[id] = clk;
>> +}
>> +
>> +static struct clk
>> +*intel_clk_register_fixed(struct intel_clk_provider *ctx,
>> + struct intel_clk_branch *list)
>> +{
>> + if (list->div_flags & CLOCK_FLAG_VAL_INIT)
>> + intel_set_clk_val(ctx->map, list->div_off, list->div_shift,
>> + list->div_width, list->div_val);
>> +
>> + return clk_register_fixed_rate(NULL, list->name, list->parent_names[0],
>> + list->flags, list->mux_flags);
>> +}
>> +
>> +static u8 intel_clk_mux_get_parent(struct clk_hw *hw)
>> +{
>> + struct intel_clk_mux *mux = to_intel_clk_mux(hw);
>> + u32 val;
>> +
>> + val = intel_get_clk_val(mux->map, mux->reg, mux->shift, mux->width);
>> + return clk_mux_val_to_index(hw, NULL, mux->flags, val);
>> +}
>> +
>> +static int intel_clk_mux_set_parent(struct clk_hw *hw, u8 index)
>> +{
>> + struct intel_clk_mux *mux = to_intel_clk_mux(hw);
>> + u32 val;
>> +
>> + val = clk_mux_index_to_val(NULL, mux->flags, index);
>> + intel_set_clk_val(mux->map, mux->reg, mux->shift, mux->width, val);
>> +
>> + return 0;
>> +}
>> +
>> +static int intel_clk_mux_determine_rate(struct clk_hw *hw,
>> + struct clk_rate_request *req)
>> +{
>> + struct intel_clk_mux *mux = to_intel_clk_mux(hw);
>> +
>> + return clk_mux_determine_rate_flags(hw, req, mux->flags);
>> +}
>> +
>> +const static struct clk_ops intel_clk_mux_ops = {
>> + .get_parent = intel_clk_mux_get_parent,
>> + .set_parent = intel_clk_mux_set_parent,
>> + .determine_rate = intel_clk_mux_determine_rate,
>> +};
>> +
>> +static struct clk
>> +*intel_clk_register_mux(struct intel_clk_provider *ctx,
>> + struct intel_clk_branch *list)
>> +{
>> + struct clk_init_data init;
>> + struct clk_hw *hw;
>> + struct intel_clk_mux *mux;
>> + u32 reg = list->mux_off;
>> + u8 shift = list->mux_shift;
>> + u8 width = list->mux_width;
>> + unsigned long cflags = list->mux_flags;
>> + int ret;
>> +
>> + mux = kzalloc(sizeof(*mux), GFP_KERNEL);
>> + if (!mux)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + init.name = list->name;
>> + init.ops = &intel_clk_mux_ops;
>> + init.flags = list->flags | CLK_IS_BASIC;
>> + init.parent_names = list->parent_names;
>> + init.num_parents = list->num_parents;
>> +
>> + mux->map = ctx->map;
>> + mux->reg = reg;
>> + mux->shift = shift;
>> + mux->width = width;
>> + mux->flags = cflags;
>> + mux->hw.init = &init;
>> +
>> + hw = &mux->hw;
>> + ret = clk_hw_register(NULL, hw);
>> + if (ret) {
>> + kfree(mux);
>> + return ERR_PTR(ret);
>> + }
>> +
>> + if (cflags & CLOCK_FLAG_VAL_INIT)
>> + intel_set_clk_val(ctx->map, reg, shift, width, list->mux_val);
>> +
>> + return hw->clk;
>> +}
>> +
>> +static unsigned long
>> +intel_clk_divider_recalc_rate(struct clk_hw *hw,
>> + unsigned long parent_rate)
>> +{
>> + struct intel_clk_divider *divider = to_intel_clk_divider(hw);
>> + unsigned int val;
>> +
>> + val = intel_get_clk_val(divider->map, divider->reg,
>> + divider->shift, divider->width);
>> + return divider_recalc_rate(hw, parent_rate, val, divider->table,
>> + divider->flags, divider->width);
>> +}
>> +
>> +static long
>> +intel_clk_divider_round_rate(struct clk_hw *hw, unsigned long rate,
>> + unsigned long *prate)
>> +{
>> + struct intel_clk_divider *divider = to_intel_clk_divider(hw);
>> +
>> + return divider_round_rate(hw, rate, prate, divider->table,
>> + divider->width, divider->flags);
>> +}
>> +
>> +static int
>> +intel_clk_divider_set_rate(struct clk_hw *hw, unsigned long rate,
>> + unsigned long prate)
>> +{
>> + struct intel_clk_divider *divider = to_intel_clk_divider(hw);
>> + int value;
>> +
>> + value = divider_get_val(rate, prate, divider->table,
>> + divider->width, divider->flags);
>> + if (value < 0)
>> + return value;
>> +
>> + intel_set_clk_val(divider->map, divider->reg,
>> + divider->shift, divider->width, value);
>> +
>> + return 0;
>> +}
>> +
>> +const static struct clk_ops intel_clk_divider_ops = {
>> + .recalc_rate = intel_clk_divider_recalc_rate,
>> + .round_rate = intel_clk_divider_round_rate,
>> + .set_rate = intel_clk_divider_set_rate,
>> +};
>> +
>> +static struct clk
>> +*intel_clk_register_divider(struct intel_clk_provider *ctx,
>> + struct intel_clk_branch *list)
>> +{
>> + struct clk_init_data init;
>> + struct clk_hw *hw;
>> + struct intel_clk_divider *div;
>> + u32 reg = list->div_off;
>> + u8 shift = list->div_shift;
>> + u8 width = list->div_width;
>> + unsigned long cflags = list->div_flags;
>> + int ret;
>> +
>> + div = kzalloc(sizeof(*div), GFP_KERNEL);
>> + if (!div)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + init.name = list->name;
>> + init.ops = &intel_clk_divider_ops;
>> + init.flags = list->flags | CLK_IS_BASIC;
>> + init.parent_names = &list->parent_names[0];
>> + init.num_parents = 1;
>> +
>> + div->map = ctx->map;
>> + div->reg = reg;
>> + div->shift = shift;
>> + div->width = width;
>> + div->flags = cflags;
>> + div->table = list->div_table;
>> + div->hw.init = &init;
>> +
>> + hw = &div->hw;
>> + ret = clk_hw_register(NULL, hw);
>> + if (ret) {
>> + pr_err("%s: register clk: %s failed!\n",
>> + __func__, list->name);
>> + kfree(div);
>> + return ERR_PTR(ret);
>> + }
>> +
>> + if (cflags & CLOCK_FLAG_VAL_INIT)
>> + intel_set_clk_val(ctx->map, reg, shift, width, list->div_val);
>> +
>> + return hw->clk;
>> +}
>> +
>> +static struct clk
>> +*intel_clk_register_fixed_factor(struct intel_clk_provider *ctx,
>> + struct intel_clk_branch *list)
>> +{
>> + struct clk_hw *hw;
>> +
>> + hw = clk_hw_register_fixed_factor(NULL, list->name,
>> + list->parent_names[0], list->flags,
>> + list->mult, list->div);
>> + if (IS_ERR(hw))
>> + return ERR_CAST(hw);
>> +
>> + if (list->div_flags & CLOCK_FLAG_VAL_INIT)
>> + intel_set_clk_val(ctx->map, list->div_off, list->div_shift,
>> + list->div_width, list->div_val);
>> +
>> + return hw->clk;
>> +}
>> +
>> +static int
>> +intel_clk_gate_enable(struct clk_hw *hw)
>> +{
>> + struct intel_clk_gate *gate = to_intel_clk_gate(hw);
>> + unsigned int reg;
>> +
>> + if (gate->flags & GATE_CLK_VT) {
>> + gate->reg = 1;
>> + return 0;
>> + }
>> +
>> + if (gate->flags & GATE_CLK_HW) {
>> + reg = GATE_HW_REG_EN(gate->reg);
>> + } else if (gate->flags & GATE_CLK_SW) {
>> + reg = gate->reg;
>> + } else {
>> + pr_err("%s: gate clk: %s: flag 0x%lx not supported!\n",
>> + __func__, clk_hw_get_name(hw), gate->flags);
>> + return 0;
>> + }
>> +
>> + intel_set_clk_val(gate->map, reg, gate->shift, 1, 1);
>> +
>> + return 0;
>> +}
>> +
>> +static void
>> +intel_clk_gate_disable(struct clk_hw *hw)
>> +{
>> + struct intel_clk_gate *gate = to_intel_clk_gate(hw);
>> + unsigned int reg;
>> + unsigned int set;
>> +
>> + if (gate->flags & GATE_CLK_VT) {
>> + gate->reg = 0;
>> + return;
>> + }
>> +
>> + if (gate->flags & GATE_CLK_HW) {
>> + reg = GATE_HW_REG_DIS(gate->reg);
>> + set = 1;
>> + } else if (gate->flags & GATE_CLK_SW) {
>> + reg = gate->reg;
>> + set = 0;
>> + } else {
>> + pr_err("%s: gate clk: %s: flag 0x%lx not supported!\n",
>> + __func__, clk_hw_get_name(hw), gate->flags);
>> + return;
>> + }
>> +
>> + intel_set_clk_val(gate->map, reg, gate->shift, 1, set);
>> +}
>> +
>> +static int
>> +intel_clk_gate_is_enabled(struct clk_hw *hw)
>> +{
>> + struct intel_clk_gate *gate = to_intel_clk_gate(hw);
>> + unsigned int reg;
>> +
>> + if (gate->flags & GATE_CLK_VT)
>> + return gate->reg;
>> +
>> + if (gate->flags & GATE_CLK_HW) {
>> + reg = GATE_HW_REG_STAT(gate->reg);
>> + } else if (gate->flags & GATE_CLK_SW) {
>> + reg = gate->reg;
>> + } else {
>> + pr_err("%s: gate clk: %s: flag 0x%lx not supported!\n",
>> + __func__, clk_hw_get_name(hw), gate->flags);
>> + return 0;
>> + }
>> +
>> + return intel_get_clk_val(gate->map, reg, gate->shift, 1);
>> +}
>> +
>> +const static struct clk_ops intel_clk_gate_ops = {
>> + .enable = intel_clk_gate_enable,
>> + .disable = intel_clk_gate_disable,
>> + .is_enabled = intel_clk_gate_is_enabled,
>> +};
>> +
>> +static struct clk
>> +*intel_clk_register_gate(struct intel_clk_provider *ctx,
>> + struct intel_clk_branch *list)
>> +{
>> + struct clk_init_data init;
> Please init the init struct with { } so that future possible additions
> to the structure don't require us to hunt this silent corruption down
> later.
Will update it.
>> + struct clk_hw *hw;
>> + struct intel_clk_gate *gate;
>> + u32 reg = list->gate_off;
>> + u8 shift = list->gate_shift;
>> + unsigned long cflags = list->gate_flags;
>> + const char *pname = list->parent_names[0];
>> + int ret;
>> +
>> + gate = kzalloc(sizeof(*gate), GFP_KERNEL);
>> + if (!gate)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + init.name = list->name;
>> + init.ops = &intel_clk_gate_ops;
>> + init.flags = list->flags | CLK_IS_BASIC;
>> + init.parent_names = pname ? &pname : NULL;
>> + init.num_parents = pname ? 1 : 0;
>> +
>> + gate->map = ctx->map;
>> + gate->reg = reg;
>> + gate->shift = shift;
>> + gate->flags = cflags;
>> + gate->hw.init = &init;
>> +
>> + hw = &gate->hw;
>> + ret = clk_hw_register(NULL, hw);
>> + if (ret) {
>> + kfree(gate);
>> + return ERR_PTR(ret);
>> + }
>> +
>> + if (cflags & CLOCK_FLAG_VAL_INIT)
>> + intel_set_clk_val(ctx->map, reg, shift, 1, list->gate_val);
>> +
>> + return hw->clk;
>> +}
>> +
>> +void intel_clk_register_branches(struct intel_clk_provider *ctx,
>> + struct intel_clk_branch *list,
>> + unsigned int nr_clk)
>> +{
>> + struct clk *clk;
>> + unsigned int idx;
>> +
>> + for (idx = 0; idx < nr_clk; idx++, list++) {
>> + switch (list->type) {
>> + case intel_clk_fixed:
> Please use uppercase for enums.
Will update.
>
>> + clk = intel_clk_register_fixed(ctx, list);
>> + break;
>> + case intel_clk_mux:
>> + clk = intel_clk_register_mux(ctx, list);
>> + break;
>> + case intel_clk_divider:
>> + clk = intel_clk_register_divider(ctx, list);
>> + break;
>> + case intel_clk_fixed_factor:
>> + clk = intel_clk_register_fixed_factor(ctx, list);
>> + break;
>> + case intel_clk_gate:
>> + clk = intel_clk_register_gate(ctx, list);
>> + break;
>> + default:
>> + pr_err("%s: type: %u not supported!\n",
>> + __func__, list->type);
>> + return;
>> + }
>> +
>> + if (IS_ERR(clk)) {
>> + pr_err("%s: register clk: %s, type: %u failed!\n",
>> + __func__, list->name, list->type);
>> + return;
>> + }
>> +
>> + intel_clk_add_lookup(ctx, clk, list->id);
>> + }
>> +}
>> +
>> +struct intel_clk_provider * __init
>> +intel_clk_init(struct device_node *np, struct regmap *map, unsigned int nr_clks)
>> +{
>> + struct intel_clk_provider *ctx;
>> + struct clk **clks;
>> +
>> + ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
>> + if (!ctx)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + clks = kcalloc(nr_clks, sizeof(*clks), GFP_KERNEL);
>> + if (!clks) {
>> + kfree(ctx);
>> + return ERR_PTR(-ENOMEM);
>> + }
>> +
>> + memset_p((void **)clks, ERR_PTR(-ENOENT), nr_clks);
>> + ctx->map = map;
>> + ctx->clk_data.clks = clks;
>> + ctx->clk_data.clk_num = nr_clks;
>> + ctx->np = np;
>> +
>> + return ctx;
>> +}
>> +
>> +void __init intel_clk_register_osc(struct intel_clk_provider *ctx,
>> + struct intel_osc_clk *osc,
>> + unsigned int nr_clks)
>> +{
>> + u32 freq;
>> + struct clk *clk;
>> + int idx;
>> +
>> + for (idx = 0; idx < nr_clks; idx++, osc++) {
>> + if (!osc->dt_freq ||
>> + of_property_read_u32(ctx->np, osc->dt_freq, &freq))
>> + freq = osc->def_rate;
>> +
>> + clk = clk_register_fixed_rate(NULL, osc->name, NULL, 0, freq);
> Should come from DT itself.
Yes. It can be defined as fixed-clock node in device tree.
Do you mean it should be defined in device tree and driver reference it
via device tree?
>> + iS_ERR(clk)) {
>> + pr_err("%s: Failed to register clock: %s\n",
>> + __func__, osc->name);
>> + return;
>> + }
>> +
>> + intel_clk_add_lookup(ctx, clk, osc->id);
>> + }
>> +}
>> diff --git a/drivers/clk/intel/clk-cgu.h b/drivers/clk/intel/clk-cgu.h
>> new file mode 100644
>> index 000000000000..6dc4e45fc499
>> --- /dev/null
>> +++ b/drivers/clk/intel/clk-cgu.h
>> @@ -0,0 +1,259 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Copyright(c) 2018 Intel Corporation.
>> + * Zhu YiXin <Yixin.zhu@intel.com>
>> + */
>> +
>> +#ifndef __INTEL_CLK_H
>> +#define __INTEL_CLK_H
>> +
>> +#define PNAME(x) static const char *const x[] __initconst
>> +
>> +struct intel_clk_mux {
>> + struct clk_hw hw;
>> + struct regmap *map;
>> + unsigned int reg;
>> + u8 shift;
>> + u8 width;
>> + unsigned long flags;
>> +};
>> +
>> +struct intel_clk_divider {
>> + struct clk_hw hw;
>> + struct regmap *map;
>> + unsigned int reg;
>> + u8 shift;
>> + u8 width;
>> + unsigned long flags;
>> + const struct clk_div_table *table;
>> +};
>> +
>> +struct intel_clk_gate {
>> + struct clk_hw hw;
>> + struct regmap *map;
>> + unsigned int reg;
>> + u8 shift;
>> + unsigned long flags;
>> +};
>> +
>> +enum intel_clk_type {
>> + intel_clk_fixed,
>> + intel_clk_mux,
>> + intel_clk_divider,
>> + intel_clk_fixed_factor,
>> + intel_clk_gate,
>> +};
>> +
>> +/**
>> + * struct intel_clk_provider
>> + * @map: regmap type base address for register.
>> + * @np: device node
>> + * @clk_data: array of hw clocks and clk number.
>> + */
>> +struct intel_clk_provider {
>> + struct regmap *map;
>> + struct device_node *np;
>> + struct clk_onecell_data clk_data;
> Please register clk_hw pointers instead of clk pointers with the of
> provider APIs.
Sorry. I'm not sure I understand you correctly.
If only registering clk_hw pointer, not registering of_provider API, then
how to reference it in the user drivers ?
Could you please give me more hints ?
>> +};
>> +
>> +/**
>> + * struct intel_pll_clk
>> + * @id: plaform specific id of the clock.
>> + * @name: name of this pll clock.
>> + * @parent_names: name of the parent clock.
>> + * @num_parents: number of parents.
>> + * @flags: optional flags for basic clock.
>> + * @type: platform type of pll.
>> + * @reg: offset of the register.
>> + * @mult: init value of mulitplier.
>> + * @div: init value of divider.
>> + * @frac: init value of fraction.
>> + * @rate_table: table of pll clock rate.
> Please drop the full-stop on kernel doc one-liners like this.
Will update it.
>
>> + */
>> +struct intel_pll_clk {
>> + unsigned int id;
>> + const char *name;
>> + const char *const *parent_names;
>> + u8 num_parents;
> Can the PLL have multiple parents?
Yes. But not in this platform.
The define here make it easy to expand to support new platform.
>> + unsigned long flags;
>> + enum intel_pll_type type;
>> + int reg;
>> + unsigned int mult;
>> + unsigned int div;
>> + unsigned int frac;
>> + const struct intel_pll_rate_table *rate_table;
>> +};
>> +
>> +#define INTEL_PLL(_id, _type, _name, _pnames, _flags, \
>> + _reg, _rtable, _mult, _div, _frac) \
>> + { \
>> + .id = _id, \
>> + .type = _type, \
>> + .name = _name, \
>> + .parent_names = _pnames, \
>> + .num_parents = ARRAY_SIZE(_pnames), \
>> + .flags = _flags, \
>> + .reg = _reg, \
>> + .rate_table = _rtable, \
>> + .mult = _mult, \
>> + .div = _div, \
>> + .frac = _frac \
>> + }
>> +
>> +/**
>> + * struct intel_osc_clk
>> + * @id: platform specific id of the clock.
>> + * @name: name of the osc clock.
>> + * @dt_freq: frequency node name in device tree.
>> + * @def_rate: default rate of the osc clock.
>> + * @flags: optional flags for basic clock.
> There aren't flags though. I'm very confused by this kernel-doc too.
> Looks like something that should be done with a fixed rate clk in DT.
Will remove the flags comments.
>> + */
>> +struct intel_osc_clk {
>> + unsigned int id;
>> + const char *name;
>> + const char *dt_freq;
>> + const u32 def_rate;
>> +};
>> +
>> +#define INTEL_OSC(_id, _name, _freq, _rate) \
>> + { \
>> + .id = _id, \
>> + .name = _name, \
>> + .dt_freq = _freq, \
>> + .def_rate = _rate, \
>> + }
>> +
>> +struct intel_clk_branch {
> Seems to be more like intel_clk instead of intel_clk_branch because it
> does lots of stuff.
Will update.
>> + unsigned int id;
>> + enum intel_clk_type type;
>> + const char *name;
>> + const char *const *parent_names;
>> + u8 num_parents;
>> + unsigned long flags;
>> + unsigned int mux_off;
>> + u8 mux_shift;
>> + u8 mux_width;
>> + unsigned long mux_flags;
>> + unsigned int mux_val;
>> + unsigned int div_off;
>> + u8 div_shift;
>> + u8 div_width;
>> + unsigned long div_flags;
>> + unsigned int div_val;
>> + const struct clk_div_table *div_table;
>> + unsigned int gate_off;
>> + u8 gate_shift;
>> + unsigned long gate_flags;
>> + unsigned int gate_val;
>> + unsigned int mult;
>> + unsigned int div;
>> +};
>> +
>> +/* clock flags definition */
>> +#define CLOCK_FLAG_VAL_INIT BIT(16)
>> +#define GATE_CLK_HW BIT(17)
>> +#define GATE_CLK_SW BIT(18)
>> +#define GATE_CLK_VT BIT(19)
> What does VT mean? Virtual?
Yes. VT means virtual here.
Will change to GATE_CLK_VIRT.
>> +
>> +#define INTEL_MUX(_id, _name, _pname, _f, _reg, \
>> + _shift, _width, _cf, _v) \
>> + { \
>> + .id = _id, \
>> + .type = intel_clk_mux, \
>> + .name = _name, \
>> + .parent_names = _pname, \
>> + .num_parents = ARRAY_SIZE(_pname), \
>> + .flags = _f, \
>> + .mux_off = _reg, \
>> + .mux_shift = _shift, \
>> + .mux_width = _width, \
>> + .mux_flags = _cf, \
>> + .mux_val = _v, \
>> + }
>> +
>> +#define INTEL_DIV(_id, _name, _pname, _f, _reg, \
>> + _shift, _width, _cf, _v, _dtable) \
>> + { \
>> + .id = _id, \
>> + .type = intel_clk_divider, \
>> + .name = _name, \
>> + .parent_names = (const char *[]) { _pname }, \
>> + .num_parents = 1, \
>> + .flags = _f, \
>> + .div_off = _reg, \
>> + .div_shift = _shift, \
>> + .div_width = _width, \
>> + .div_flags = _cf, \
>> + .div_val = _v, \
>> + .div_table = _dtable, \
>> + }
>> +
>> +#define INTEL_GATE(_id, _name, _pname, _f, _reg, \
>> + _shift, _cf, _v) \
>> + { \
>> + .id = _id, \
>> + .type = intel_clk_gate, \
>> + .name = _name, \
>> + .parent_names = (const char *[]) { _pname }, \
>> + .num_parents = !_pname ? 0 : 1, \
>> + .flags = _f, \
>> + .gate_off = _reg, \
>> + .gate_shift = _shift, \
>> + .gate_flags = _cf, \
>> + .gate_val = _v, \
>> + }
>> +
>> +#define INTEL_FIXED(_id, _name, _pname, _f, _reg, \
>> + _shift, _width, _cf, _freq, _v) \
>> + { \
>> + .id = _id, \
>> + .type = intel_clk_fixed, \
>> + .name = _name, \
>> + .parent_names = (const char *[]) { _pname }, \
>> + .num_parents = !_pname ? 0 : 1, \
>> + .flags = _f, \
>> + .div_off = _reg, \
>> + .div_shift = _shift, \
>> + .div_width = _width, \
>> + .div_flags = _cf, \
>> + .div_val = _v, \
>> + .mux_flags = _freq, \
>> + }
>> +
>> +#define INTEL_FIXED_FACTOR(_id, _name, _pname, _f, _reg, \
>> + _shift, _width, _cf, _v, _m, _d) \
>> + { \
>> + .id = _id, \
>> + .type = intel_clk_fixed_factor, \
>> + .name = _name, \
>> + .parent_names = (const char *[]) { _pname }, \
>> + .num_parents = 1, \
>> + .flags = _f, \
>> + .div_off = _reg, \
>> + .div_shift = _shift, \
>> + .div_width = _width, \
>> + .div_flags = _cf, \
>> + .div_val = _v, \
>> + .mult = _m, \
>> + .div = _d, \
>> + }
>> +
>> +void intel_set_clk_val(struct regmap *map, u32 reg, u8 shift,
>> + u8 width, u32 set_val);
>> +u32 intel_get_clk_val(struct regmap *map, u32 reg, u8 shift, u8 width);
>> +void intel_clk_add_lookup(struct intel_clk_provider *ctx,
>> + struct clk *clk, unsigned int id);
>> +void __init intel_clk_of_add_provider(struct device_node *np,
>> + struct intel_clk_provider *ctx);
>> +struct intel_clk_provider * __init
>> +intel_clk_init(struct device_node *np, struct regmap *map,
>> + unsigned int nr_clks);
>> +void __init intel_clk_register_osc(struct intel_clk_provider *ctx,
>> + struct intel_osc_clk *osc,
>> + unsigned int nr_clks);
> Remove __init from headers files. It does nothing.
Will remove it.
>
>> +void intel_clk_register_branches(struct intel_clk_provider *ctx,
>> + struct intel_clk_branch *list,
>> + unsigned int nr_clk);
>> +void intel_clk_register_plls(struct intel_clk_provider *ctx,
>> + struct intel_pll_clk *list, unsigned int nr_clk);
>> +#endif /* __INTEL_CLK_H */
>> diff --git a/drivers/clk/intel/clk-grx500.c b/drivers/clk/intel/clk-grx500.c
>> new file mode 100644
>> index 000000000000..5c2546f82579
>> --- /dev/null
>> +++ b/drivers/clk/intel/clk-grx500.c
>> @@ -0,0 +1,168 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2018 Intel Corporation.
>> + * Zhu YiXin <Yixin.zhu@intel.com>
>> + */
>> +
>> +#include <linux/clk-provider.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/of.h>
>> +#include <linux/of_address.h>
>> +#include <linux/regmap.h>
>> +#include <linux/spinlock.h>
> Used?
Will remove it
>> +#include <dt-bindings/clock/intel,grx500-clk.h>
>> +
>> +#include "clk-cgu-pll.h"
>> +#include "clk-cgu.h"
>> +
>> +#define PLL_DIV_WIDTH 4
>> +
>> +/* Gate1 clock shift */
>> +#define G_VCODEC_SHIFT 2
>> +#define G_DMA0_SHIFT 5
>> +#define G_USB0_SHIFT 6
>> +#define G_SPI1_SHIFT 7
>> +#define G_SPI0_SHIFT 8
>> +#define G_CBM_SHIFT 9
>> +#define G_EBU_SHIFT 10
>> +#define G_SSO_SHIFT 11
>> +#define G_GPTC0_SHIFT 12
>> +#define G_GPTC1_SHIFT 13
>> +#define G_GPTC2_SHIFT 14
>> +#define G_UART_SHIFT 17
>> +#define G_CPYTO_SHIFT 20
>> +#define G_SECPT_SHIFT 21
>> +#define G_TOE_SHIFT 22
>> +#define G_MPE_SHIFT 23
>> +#define G_TDM_SHIFT 25
>> +#define G_PAE_SHIFT 26
>> +#define G_USB1_SHIFT 27
>> +#define G_SWITCH_SHIFT 28
>> +
>> +/* Gate2 clock shift */
>> +#define G_PCIE0_SHIFT 1
>> +#define G_PCIE1_SHIFT 17
>> +#define G_PCIE2_SHIFT 25
>> +
>> +/* Register definition */
>> +#define GRX500_PLL0A_CFG0 0x0004
>> +#define GRX500_PLL0A_CFG1 0x0008
>> +#define GRX500_PLL0B_CFG0 0x0034
>> +#define GRX500_PLL0B_CFG1 0x0038
>> +#define GRX500_LCPLL_CFG0 0x0094
>> +#define GRX500_LCPLL_CFG1 0x0098
>> +#define GRX500_IF_CLK 0x00c4
>> +#define GRX500_CLK_GSR1 0x0120
>> +#define GRX500_CLK_GSR2 0x0130
>> +
>> +static const struct clk_div_table pll_div[] = {
>> + {1, 2},
> Please write it like
>
> { 1, 2 },
>
> instead.
Will update.
>> + {2, 3},
>> + {3, 4},
>> + {4, 5},
>> + {5, 6},
>> + {6, 8},
>> + {7, 10},
>> + {8, 12},
>> + {9, 16},
>> + {10, 20},
>> + {11, 24},
>> + {12, 32},
>> + {13, 40},
>> + {14, 48},
>> + {15, 64}
>> +};
>> +
>> +enum grx500_plls {
>> + pll0a, pll0b, pll3,
>> +};
> What's the point of the enum?
Will remove it.
>> +
>> +PNAME(pll_p) = { "osc" };
>> +PNAME(cpu_p) = { "cpu0", "cpu1" };
>> +
>> +static struct intel_osc_clk grx500_osc_clks[] __initdata = {
>> + INTEL_OSC(CLK_OSC, "osc", "intel,osc-frequency", 40000000),
>> +};
>> +
>> +static struct intel_pll_clk grx500_pll_clks[] __initdata = {
>> + [pll0a] = INTEL_PLL(CLK_PLL0A, pll_grx500, "pll0a",
>> + pll_p, 0, GRX500_PLL0A_CFG0, NULL, 0, 0, 0),
>> + [pll0b] = INTEL_PLL(CLK_PLL0B, pll_grx500, "pll0b",
>> + pll_p, 0, GRX500_PLL0B_CFG0, NULL, 0, 0, 0),
>> + [pll3] = INTEL_PLL(CLK_PLL3, pll_grx500, "pll3",
>> + pll_p, 0, GRX500_LCPLL_CFG0, NULL, 0, 0, 0),
>> +};
>> +
>> +static struct intel_clk_branch grx500_branch_clks[] __initdata = {
>> + INTEL_DIV(CLK_CBM, "cbm", "pll0a", 0, GRX500_PLL0A_CFG1,
>> + 0, PLL_DIV_WIDTH, 0, 0, pll_div),
>> + INTEL_DIV(CLK_NGI, "ngi", "pll0a", 0, GRX500_PLL0A_CFG1,
>> + 4, PLL_DIV_WIDTH, 0, 0, pll_div),
>> + INTEL_DIV(CLK_SSX4, "ssx4", "pll0a", 0, GRX500_PLL0A_CFG1,
>> + 8, PLL_DIV_WIDTH, 0, 0, pll_div),
>> + INTEL_DIV(CLK_CPU0, "cpu0", "pll0a", 0, GRX500_PLL0A_CFG1,
>> + 12, PLL_DIV_WIDTH, 0, 0, pll_div),
>> + INTEL_DIV(CLK_PAE, "pae", "pll0b", 0, GRX500_PLL0B_CFG1,
>> + 0, PLL_DIV_WIDTH, 0, 0, pll_div),
>> + INTEL_DIV(CLK_GSWIP, "gswip", "pll0b", 0, GRX500_PLL0B_CFG1,
>> + 4, PLL_DIV_WIDTH, 0, 0, pll_div),
>> + INTEL_DIV(CLK_DDR, "ddr", "pll0b", 0, GRX500_PLL0B_CFG1,
>> + 8, PLL_DIV_WIDTH, 0, 0, pll_div),
>> + INTEL_DIV(CLK_CPU1, "cpu1", "pll0b", 0, GRX500_PLL0B_CFG1,
>> + 12, PLL_DIV_WIDTH, 0, 0, pll_div),
>> + INTEL_MUX(CLK_CPU, "cpu", cpu_p, CLK_SET_RATE_PARENT,
>> + GRX500_PLL0A_CFG1, 29, 1, 0, 0),
>> + INTEL_GATE(GCLK_DMA0, "g_dma0", NULL, 0, GRX500_CLK_GSR1,
>> + G_DMA0_SHIFT, GATE_CLK_HW, 0),
>> + INTEL_GATE(GCLK_USB0, "g_usb0", NULL, 0, GRX500_CLK_GSR1,
>> + G_USB0_SHIFT, GATE_CLK_HW, 0),
>> + INTEL_GATE(GCLK_GPTC0, "g_gptc0", NULL, 0, GRX500_CLK_GSR1,
>> + G_GPTC0_SHIFT, GATE_CLK_HW, 0),
>> + INTEL_GATE(GCLK_GPTC1, "g_gptc1", NULL, 0, GRX500_CLK_GSR1,
>> + G_GPTC1_SHIFT, GATE_CLK_HW, 0),
>> + INTEL_GATE(GCLK_GPTC2, "g_gptc2", NULL, 0, GRX500_CLK_GSR1,
>> + G_GPTC2_SHIFT, GATE_CLK_HW, 0),
>> + INTEL_GATE(GCLK_UART, "g_uart", NULL, 0, GRX500_CLK_GSR1,
>> + G_UART_SHIFT, GATE_CLK_HW, 0),
>> + INTEL_GATE(GCLK_PCIE0, "g_pcie0", NULL, 0, GRX500_CLK_GSR2,
>> + G_PCIE0_SHIFT, GATE_CLK_HW, 0),
>> + INTEL_GATE(GCLK_PCIE1, "g_pcie1", NULL, 0, GRX500_CLK_GSR2,
>> + G_PCIE1_SHIFT, GATE_CLK_HW, 0),
>> + INTEL_GATE(GCLK_PCIE2, "g_pcie2", NULL, 0, GRX500_CLK_GSR2,
>> + G_PCIE2_SHIFT, GATE_CLK_HW, 0),
>> + INTEL_GATE(GCLK_I2C, "g_i2c", NULL, 0, 0, 0, GATE_CLK_VT, 0),
>> + INTEL_FIXED(CLK_VOICE, "voice", NULL, 0, GRX500_IF_CLK, 14, 2,
>> + CLOCK_FLAG_VAL_INIT, 8192000, 2),
>> + INTEL_FIXED_FACTOR(CLK_DDRPHY, "ddrphy", "ddr", 0, 0, 0,
>> + 0, 0, 0, 2, 1),
>> + INTEL_FIXED_FACTOR(CLK_PCIE, "pcie", "pll3", 0, 0, 0,
>> + 0, 0, 0, 1, 40),
>> +};
>> +
>> +static void __init grx500_clk_init(struct device_node *np)
>> +{
>> + struct intel_clk_provider *ctx;
>> + struct regmap *map;
>> +
>> + map = syscon_node_to_regmap(np);
>> + if (IS_ERR(map))
>> + return;
>> +
>> + ctx = intel_clk_init(np, map, CLK_NR_CLKS);
>> + if (IS_ERR(ctx)) {
>> + regmap_exit(map);
>> + return;
>> + }
>> +
>> + intel_clk_register_osc(ctx, grx500_osc_clks,
>> + ARRAY_SIZE(grx500_osc_clks));
>> + intel_clk_register_plls(ctx, grx500_pll_clks,
>> + ARRAY_SIZE(grx500_pll_clks));
>> + intel_clk_register_branches(ctx, grx500_branch_clks,
>> + ARRAY_SIZE(grx500_branch_clks));
>> + of_clk_add_provider(np, of_clk_src_onecell_get, &ctx->clk_data);
>> +
>> + pr_debug("%s clk init done!\n", __func__);
> Yay!!!
>
>> +}
>> +
>> +CLK_OF_DECLARE(intel_grx500_cgu, "intel,grx500-cgu", grx500_clk_init);
> Any reason a platform driver can't be used instead of CLK_OF_DECLARE()?
It provides CPU clock which is used in early boot stage.
^ permalink raw reply
* Re: [PATCH v2 08/18] serial: intel: Get serial id from dts
From: Geert Uytterhoeven @ 2018-08-08 8:33 UTC (permalink / raw)
To: songjun.wu
Cc: hua.ma, yixin.zhu, chuanhua.lei, qi-ming.wu,
Linux MIPS Mailing List, linux-clk, open list:SERIAL DRIVERS,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Greg KH, Linux Kernel Mailing List, Jiri Slaby
In-Reply-To: <734bddbc-a5aa-d50d-0e7b-d8adc4d1afb2@linux.intel.com>
Hi Songjun,
On Wed, Aug 8, 2018 at 6:05 AM Wu, Songjun <songjun.wu@linux.intel.com> wrote:
> On 8/7/2018 3:33 PM, Geert Uytterhoeven wrote:
> > On Fri, Aug 3, 2018 at 5:04 AM Songjun Wu <songjun.wu@linux.intel.com> wrote:
> >> Get serial id from dts.
> >>
> >> "#ifdef CONFIG_LANTIQ" preprocessor is used because LTQ_EARLY_ASC
> >> macro is defined in lantiq_soc.h.
> >> lantiq_soc.h is in arch path for legacy product support.
> >>
> >> arch/mips/include/asm/mach-lantiq/xway/lantiq_soc.h
> >>
> >> If "#ifdef preprocessor" is changed to
> >> "if (IS_ENABLED(CONFIG_LANTIQ))", when CONFIG_LANTIQ is not enabled,
> >> code using LTQ_EARLY_ASC is compiled.
> >> Compilation will fail for no LTQ_EARLY_ASC defined.
> >>
> >> Signed-off-by: Songjun Wu <songjun.wu@linux.intel.com>
> > Thanks for your patch!
> >
> >> @@ -699,9 +700,19 @@ lqasc_probe(struct platform_device *pdev)
> >> return -ENODEV;
> >> }
> >>
> >> - /* check if this is the console port */
> >> - if (mmres->start != CPHYSADDR(LTQ_EARLY_ASC))
> >> - line = 1;
> >> + /* get serial id */
> >> + line = of_alias_get_id(node, "serial");
> >> + if (line < 0) {
> >> +#ifdef CONFIG_LANTIQ
> >> + if (mmres->start == CPHYSADDR(LTQ_EARLY_ASC))
> >> + line = 0;
> >> + else
> >> + line = 1;
> >> +#else
> >> + dev_err(&pdev->dev, "failed to get alias id, errno %d\n", line);
> >> + return line;
> > Please note that not providing a fallback here makes life harder when using
> > DT overlays.
> > See the description of commit 7678f4c20fa7670f ("serial: sh-sci: Add support
> > for dynamic instances") for background info.
> Thanks for your comment.
> The logic in commit 7678f4c20fa7670f is not suitable here.
> We need to know which serial instance is used for console.
> We cannot use dynamic serial instance here.
Why does the driver need to use which serial instance is used for the console?
Hardcoding that is not an option, as the board DTS may specify the console using
chosen/stdout-path.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: Re: [RFC PATCH 5/6] arm64: dts: ti: Add Support for AM654 SoC
From: Kishon Vijay Abraham I @ 2018-08-08 6:31 UTC (permalink / raw)
To: Rob Herring, Nishanth Menon
Cc: Santosh Shilimkar, Will Deacon, Catalin Marinas,
Greg Kroah-Hartman, Mark Rutland, open list:SERIAL DRIVERS,
linux-kernel@vger.kernel.org, devicetree,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Tony Lindgren, Vignesh R, Tero Kristo, Russell King, Sudeep Holla
In-Reply-To: <CAL_JsqJusWTvr4A_-Bk81meYddMHBMJ4=Fch6L0MFoF7HfBW2w@mail.gmail.com>
Hi Rob,
On Tuesday 05 June 2018 07:35 PM, Rob Herring wrote:
> On Tue, Jun 5, 2018 at 1:05 AM, Nishanth Menon <nm@ti.com> wrote:
>> The AM654 SoC is a lead device of the K3 Multicore SoC architecture
>> platform, targeted for broad market and industrial control with aim to
>> meet the complex processing needs of modern embedded products.
>>
>> Some highlights of this SoC are:
>> * Quad ARMv8 A53 cores split over two clusters
>> * GICv3 compliant GIC500
>> * Configurable L3 Cache and IO-coherent architecture
>> * Dual lock-step capable R5F uC for safety-critical applications
>> * High data throughput capable distributed DMA architecture under NAVSS
>> * Three Gigabit Industrial Communication Subsystems (ICSSG), each with dual
>> PRUs and dual RTUs
>> * Hardware accelerator block containing AES/DES/SHA/MD5 called SA2UL
>> * Centralized System Controller for Security, Power, and Resource
>> management.
>> * Dual ADCSS, eQEP/eCAP, eHRPWM, dual CAN-FD
>> * Flash subystem with OSPI and Hyperbus interfaces
>> * Multimedia capability with CAL, DSS7-UL, SGX544, McASP
>> * Peripheral connectivity including USB3, PCIE, MMC/SD, GPMC, I2C, SPI,
>> GPIO
>>
>> See AM65x Technical Reference Manual (SPRUID7, April 2018)
>> for further details: http://www.ti.com/lit/pdf/spruid7
>>
>> We introduce the Kconfig symbol for the SoC along with this patch since
>> it is logically relevant point, however the usage is in subsequent
>> patches.
>>
>> NOTE: AM654 is the first of the device variants, hence we introduce a
>> generic am6.dtsi.
>>
>> Signed-off-by: Benjamin Fair <b-fair@ti.com>
>> Signed-off-by: Nishanth Menon <nm@ti.com>
>> ---
>> MAINTAINERS | 1 +
>> arch/arm64/boot/dts/ti/k3-am6.dtsi | 144 +++++++++++++++++++++++++++++++++++
>> arch/arm64/boot/dts/ti/k3-am654.dtsi | 117 ++++++++++++++++++++++++++++
>> drivers/soc/ti/Kconfig | 14 ++++
>> 4 files changed, 276 insertions(+)
>> create mode 100644 arch/arm64/boot/dts/ti/k3-am6.dtsi
>> create mode 100644 arch/arm64/boot/dts/ti/k3-am654.dtsi
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index cfb35b252ac7..5f5c4eddec7a 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -2092,6 +2092,7 @@ M: Nishanth Menon <nm@ti.com>
>> L: linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
>> S: Supported
>> F: Documentation/devicetree/bindings/arm/ti/k3.txt
>> +F: arch/arm64/boot/dts/ti/k3-*
>>
>> ARM/TEXAS INSTRUMENT KEYSTONE ARCHITECTURE
>> M: Santosh Shilimkar <ssantosh@kernel.org>
>> diff --git a/arch/arm64/boot/dts/ti/k3-am6.dtsi b/arch/arm64/boot/dts/ti/k3-am6.dtsi
>> new file mode 100644
>> index 000000000000..cdfa12173aac
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/ti/k3-am6.dtsi
>> @@ -0,0 +1,144 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Device Tree Source for AM6 SoC Family
>> + *
>> + * Copyright (C) 2016-2018 Texas Instruments Incorporated - http://www.ti.com/
>> + */
>> +
>> +#include <dt-bindings/gpio/gpio.h>
>> +#include <dt-bindings/interrupt-controller/irq.h>
>> +#include <dt-bindings/interrupt-controller/arm-gic.h>
>> +
>> +/ {
>> + model = "Texas Instruments K3 AM654 SoC";
>> + compatible = "ti,am654";
>> + interrupt-parent = <&gic>;
>> + #address-cells = <2>;
>> + #size-cells = <2>;
>> +
>> + aliases {
>> + serial0 = &wkup_uart0;
>> + serial1 = &mcu_uart0;
>> + serial2 = &main_uart0;
>> + serial3 = &main_uart1;
>> + serial4 = &main_uart2;
>> + };
>> +
>> + chosen { };
>> +
>> + firmware {
>> + optee {
>> + compatible = "linaro,optee-tz";
>> + method = "smc";
>> + };
>> +
>> + psci: psci {
>> + compatible = "arm,psci-1.0";
>> + method = "smc";
>> + };
>> + };
>> +
>> + soc0: soc0 {
>> + compatible = "simple-bus";
>> + #address-cells = <2>;
>> + #size-cells = <2>;
>> + ranges;
>
> Really need 64-bit addresses and sizes? Use ranges to limit the
> address space if possible.
We now have address-cells as <1>,
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm64/boot/dts/ti/k3-am65.dtsi#n49
However each PCIe instance has 2 data regions and one of the regions
(PCIE0_CORE_CORE_DAT_SLV_PCIE_DAT1/PCIE1_CORE_CORE_DAT_SLV_PCIE_DAT1 specified
in the "MAIN Domain Memory Map" table of TRM http://www.ti.com/lit/pdf/spruid7)
is above the 32bit region and requires 2 cells to specify the start address.
This region is used to access MEM_SPACE of PCIe endpoint when operating in root
complex mode and access memory of PCI root complex when operating in endpoint mode.
In order to describe this, should we change the address-cells back to <2> or do
you suggest any other alternatives?
Thanks
Kishon
^ permalink raw reply
* Re: [PATCH v2 02/18] clk: intel: Add clock driver for Intel MIPS SoCs
From: Stephen Boyd @ 2018-08-08 5:50 UTC (permalink / raw)
To: chuanhua.lei, hua.ma, qi-ming.wu, yixin.zhu
Cc: linux-mips, linux-clk, linux-serial, devicetree, Songjun Wu,
Michael Turquette, linux-kernel, Rob Herring, Mark Rutland
In-Reply-To: <20180803030237.3366-3-songjun.wu@linux.intel.com>
Quoting Songjun Wu (2018-08-02 20:02:21)
> From: Yixin Zhu <yixin.zhu@linux.intel.com>
>
> This driver provides PLL clock registration as well as various clock
> branches, e.g. MUX clock, gate clock, divider clock and so on.
>
> PLLs that provide clock to DDR, CPU and peripherals are shown below:
>
> +---------+
> |--->| LCPLL3 0|--PCIe clk-->
> XO | +---------+
> +-----------|
> | +---------+
> | | 3|--PAE clk-->
> |--->| PLL0B 2|--GSWIP clk-->
> | | 1|--DDR clk-->DDR PHY clk-->
> | | 0|--CPU1 clk--+ +-----+
> | +---------+ |--->0 |
> | | MUX |--CPU clk-->
> | +---------+ |--->1 |
> | | 0|--CPU0 clk--+ +-----+
> |--->| PLLOA 1|--SSX4 clk-->
> | 2|--NGI clk-->
> | 3|--CBM clk-->
> +---------+
Thanks for the picture!
>
> Signed-off-by: Yixin Zhu <yixin.zhu@linux.intel.com>
> Signed-off-by: Songjun Wu <songjun.wu@linux.intel.com>
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index 0bb25dd009d1..d929ca4607cf 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -72,6 +72,9 @@ obj-$(CONFIG_ARCH_HISI) += hisilicon/
> obj-y += imgtec/
> obj-$(CONFIG_ARCH_MXC) += imx/
> obj-$(CONFIG_MACH_INGENIC) += ingenic/
> +ifeq ($(CONFIG_COMMON_CLK), y)
> +obj-y +=intel/
> +endif
Why not obj-$(CONFIG_INTEL_CCF) or something like that?
> obj-$(CONFIG_ARCH_KEYSTONE) += keystone/
> obj-$(CONFIG_MACH_LOONGSON32) += loongson1/
> obj-y += mediatek/
> diff --git a/drivers/clk/intel/Kconfig b/drivers/clk/intel/Kconfig
> new file mode 100644
> index 000000000000..c7d3fb1721fa
> --- /dev/null
> +++ b/drivers/clk/intel/Kconfig
> @@ -0,0 +1,20 @@
> +# SPDX-License-Identifier: GPL-2.0
> +config INTEL_CGU_CLK
> + depends on COMMON_CLK
> + depends on INTEL_MIPS || COMPILE_TEST
> + select MFD_SYSCON
> + bool "Intel clock controller support"
> + help
> + This driver support Intel CGU (Clock Generation Unit).
Is it really called a clock generation unit? Or that's just copied from
sunxi driver?
> +
> +choice
> + prompt "SoC platform selection"
> + depends on INTEL_CGU_CLK
> + default INTEL_GRX500_CGU_CLK
> +
> +config INTEL_GRX500_CGU_CLK
> + bool "GRX500 CLK"
> + help
> + Clock driver of GRX500 platform.
> +
> +endchoice
> diff --git a/drivers/clk/intel/Makefile b/drivers/clk/intel/Makefile
> new file mode 100644
> index 000000000000..16a0138e52c2
> --- /dev/null
> +++ b/drivers/clk/intel/Makefile
> @@ -0,0 +1,7 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# Makefile for intel specific clk
> +
> +obj-$(CONFIG_INTEL_CGU_CLK) += clk-cgu.o clk-cgu-pll.o
> +ifneq ($(CONFIG_INTEL_GRX500_CGU_CLK),)
> + obj-y += clk-grx500.o
> +endif
> diff --git a/drivers/clk/intel/clk-cgu-pll.c b/drivers/clk/intel/clk-cgu-pll.c
> new file mode 100644
> index 000000000000..20759bc27e95
> --- /dev/null
> +++ b/drivers/clk/intel/clk-cgu-pll.c
> @@ -0,0 +1,166 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2018 Intel Corporation.
> + * Zhu YiXin <Yixin.zhu@intel.com>
> + */
> +
> +#include <linux/clk.h>
Is this include used?
> +#include <linux/clk-provider.h>
> +#include <linux/clkdev.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +
> +#include "clk-cgu-pll.h"
> +#include "clk-cgu.h"
> +
> +#define to_intel_clk_pll(_hw) container_of(_hw, struct intel_clk_pll, hw)
> +
> +/*
> + * Calculate formula:
> + * rate = (prate * mult + (prate * frac) / frac_div) / div
> + */
> +static unsigned long
> +intel_pll_calc_rate(unsigned long prate, unsigned int mult,
> + unsigned int div, unsigned int frac,
> + unsigned int frac_div)
> +{
> + u64 crate, frate, rate64;
> +
> + rate64 = prate;
> + crate = rate64 * mult;
> +
> + if (frac) {
> + frate = rate64 * frac;
> + do_div(frate, frac_div);
> + crate += frate;
> + }
> + do_div(crate, div);
> +
> + return (unsigned long)crate;
> +}
> +
> +static void
> +grx500_pll_get_params(struct intel_clk_pll *pll, unsigned int *mult,
> + unsigned int *frac)
> +{
> + *mult = intel_get_clk_val(pll->map, pll->reg, 2, 7);
> + *frac = intel_get_clk_val(pll->map, pll->reg, 9, 21);
> +}
> +
> +static int intel_wait_pll_lock(struct intel_clk_pll *pll, int bit_idx)
> +{
> + unsigned int val;
> +
> + return regmap_read_poll_timeout(pll->map, pll->reg, val,
> + val & BIT(bit_idx), 10, 1000);
> +}
> +
> +static unsigned long
> +intel_grx500_pll_recalc_rate(struct clk_hw *hw, unsigned long prate)
> +{
> + struct intel_clk_pll *pll = to_intel_clk_pll(hw);
> + unsigned int mult, frac;
> +
> + grx500_pll_get_params(pll, &mult, &frac);
> +
> + return intel_pll_calc_rate(prate, mult, 1, frac, BIT(20));
> +}
> +
> +static int intel_grx500_pll_is_enabled(struct clk_hw *hw)
> +{
> + struct intel_clk_pll *pll = to_intel_clk_pll(hw);
> +
> + if (intel_wait_pll_lock(pll, 1)) {
> + pr_err("%s: pll: %s is not locked!\n",
> + __func__, clk_hw_get_name(hw));
> + return 0;
> + }
> +
> + return intel_get_clk_val(pll->map, pll->reg, 1, 1);
> +}
> +
> +const static struct clk_ops intel_grx500_pll_ops = {
Should be static const struct ...
> + .recalc_rate = intel_grx500_pll_recalc_rate,
> + .is_enabled = intel_grx500_pll_is_enabled,
> +};
> +
> +static struct clk
> +*intel_clk_register_pll(struct intel_clk_provider *ctx,
> + enum intel_pll_type type, const char *cname,
> + const char *const *pname, u8 num_parents,
> + unsigned long flags, unsigned int reg,
> + const struct intel_pll_rate_table *table,
> + unsigned int mult, unsigned int div, unsigned int frac)
> +{
> + struct clk_init_data init;
> + struct intel_clk_pll *pll;
> + struct clk_hw *hw;
> + int ret, i;
> +
> + if (type != pll_grx500) {
> + pr_err("%s: pll type %d not supported!\n",
> + __func__, type);
> + return ERR_PTR(-EINVAL);
> + }
> + init.name = cname;
> + init.ops = &intel_grx500_pll_ops;
> + init.flags = CLK_IS_BASIC;
Don't use this flag unless you have some reason to need it.
> + init.parent_names = pname;
> + init.num_parents = num_parents;
> +
> + pll = kzalloc(sizeof(*pll), GFP_KERNEL);
> + if (!pll)
> + return ERR_PTR(-ENOMEM);
> + pll->map = ctx->map;
> + pll->reg = reg;
> + pll->flags = flags;
> + pll->mult = mult;
> + pll->div = div;
> + pll->frac = frac;
> + pll->hw.init = &init;
> + if (table) {
> + for (i = 0; table[i].rate != 0; i++)
> + ;
> + pll->table_sz = i;
> + pll->rate_table = kmemdup(table, i * sizeof(table[0]),
> + GFP_KERNEL);
> + if (!pll->rate_table) {
> + ret = -ENOMEM;
> + goto err_free_pll;
> + }
> + }
> + hw = &pll->hw;
> + ret = clk_hw_register(NULL, hw);
> + if (ret)
> + goto err_free_pll;
> +
> + return hw->clk;
> +
> +err_free_pll:
> + kfree(pll);
> + return ERR_PTR(ret);
> +}
> +
> +void intel_clk_register_plls(struct intel_clk_provider *ctx,
> + struct intel_pll_clk *list, unsigned int nr_clk)
> +{
> + struct clk *clk;
> + int i;
> +
> + for (i = 0; i < nr_clk; i++, list++) {
> + clk = intel_clk_register_pll(ctx, list->type, list->name,
> + list->parent_names, list->num_parents,
> + list->flags, list->reg, list->rate_table,
> + list->mult, list->div, list->frac);
> + if (IS_ERR(clk)) {
> + pr_err("%s: failed to register pll: %s\n",
> + __func__, list->name);
> + continue;
> + }
> +
> + intel_clk_add_lookup(ctx, clk, list->id);
> + }
> +}
> diff --git a/drivers/clk/intel/clk-cgu-pll.h b/drivers/clk/intel/clk-cgu-pll.h
> new file mode 100644
> index 000000000000..3e7cff1d5e16
> --- /dev/null
> +++ b/drivers/clk/intel/clk-cgu-pll.h
> @@ -0,0 +1,34 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright(c) 2018 Intel Corporation.
> + * Zhu YiXin <Yixin.zhu@intel.com>
> + */
> +
> +#ifndef __INTEL_CLK_PLL_H
> +#define __INTEL_CLK_PLL_H
> +
> +enum intel_pll_type {
> + pll_grx500,
> +};
> +
> +struct intel_pll_rate_table {
> + unsigned long prate;
> + unsigned long rate;
> + unsigned int mult;
> + unsigned int div;
> + unsigned int frac;
> +};
> +
> +struct intel_clk_pll {
> + struct clk_hw hw;
> + struct regmap *map;
> + unsigned int reg;
> + unsigned long flags;
> + unsigned int mult;
> + unsigned int div;
> + unsigned int frac;
> + unsigned int table_sz;
> + const struct intel_pll_rate_table *rate_table;
> +};
> +
> +#endif /* __INTEL_CLK_PLL_H */
> diff --git a/drivers/clk/intel/clk-cgu.c b/drivers/clk/intel/clk-cgu.c
> new file mode 100644
> index 000000000000..10cacbe0fbcd
> --- /dev/null
> +++ b/drivers/clk/intel/clk-cgu.c
> @@ -0,0 +1,470 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2018 Intel Corporation.
> + * Zhu YiXin <Yixin.zhu@intel.com>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <linux/clkdev.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +
> +#include "clk-cgu-pll.h"
> +#include "clk-cgu.h"
> +
> +#define GATE_HW_REG_STAT(reg) (reg)
> +#define GATE_HW_REG_EN(reg) ((reg) + 0x4)
> +#define GATE_HW_REG_DIS(reg) ((reg) + 0x8)
> +
> +#define to_intel_clk_mux(_hw) container_of(_hw, struct intel_clk_mux, hw)
> +#define to_intel_clk_divider(_hw) \
> + container_of(_hw, struct intel_clk_divider, hw)
> +#define to_intel_clk_gate(_hw) container_of(_hw, struct intel_clk_gate, hw)
> +
> +void intel_set_clk_val(struct regmap *map, u32 reg, u8 shift,
> + u8 width, u32 set_val)
> +{
> + u32 mask = GENMASK(width + shift, shift);
> +
> + regmap_update_bits(map, reg, mask, set_val << shift);
> +}
> +
> +u32 intel_get_clk_val(struct regmap *map, u32 reg, u8 shift,
> + u8 width)
> +{
> + u32 val;
> +
> + if (regmap_read(map, reg, &val)) {
> + WARN_ONCE(1, "Failed to read clk reg: 0x%x\n", reg);
> + return 0;
> + }
> + val >>= shift;
> + val &= BIT(width) - 1;
> +
> + return val;
> +}
> +
> +void intel_clk_add_lookup(struct intel_clk_provider *ctx,
> + struct clk *clk, unsigned int id)
> +{
> + pr_debug("Add clk: %s, id: %u\n", __clk_get_name(clk), id);
> + if (ctx->clk_data.clks && id)
> + ctx->clk_data.clks[id] = clk;
> +}
> +
> +static struct clk
> +*intel_clk_register_fixed(struct intel_clk_provider *ctx,
> + struct intel_clk_branch *list)
> +{
> + if (list->div_flags & CLOCK_FLAG_VAL_INIT)
> + intel_set_clk_val(ctx->map, list->div_off, list->div_shift,
> + list->div_width, list->div_val);
> +
> + return clk_register_fixed_rate(NULL, list->name, list->parent_names[0],
> + list->flags, list->mux_flags);
> +}
> +
> +static u8 intel_clk_mux_get_parent(struct clk_hw *hw)
> +{
> + struct intel_clk_mux *mux = to_intel_clk_mux(hw);
> + u32 val;
> +
> + val = intel_get_clk_val(mux->map, mux->reg, mux->shift, mux->width);
> + return clk_mux_val_to_index(hw, NULL, mux->flags, val);
> +}
> +
> +static int intel_clk_mux_set_parent(struct clk_hw *hw, u8 index)
> +{
> + struct intel_clk_mux *mux = to_intel_clk_mux(hw);
> + u32 val;
> +
> + val = clk_mux_index_to_val(NULL, mux->flags, index);
> + intel_set_clk_val(mux->map, mux->reg, mux->shift, mux->width, val);
> +
> + return 0;
> +}
> +
> +static int intel_clk_mux_determine_rate(struct clk_hw *hw,
> + struct clk_rate_request *req)
> +{
> + struct intel_clk_mux *mux = to_intel_clk_mux(hw);
> +
> + return clk_mux_determine_rate_flags(hw, req, mux->flags);
> +}
> +
> +const static struct clk_ops intel_clk_mux_ops = {
> + .get_parent = intel_clk_mux_get_parent,
> + .set_parent = intel_clk_mux_set_parent,
> + .determine_rate = intel_clk_mux_determine_rate,
> +};
> +
> +static struct clk
> +*intel_clk_register_mux(struct intel_clk_provider *ctx,
> + struct intel_clk_branch *list)
> +{
> + struct clk_init_data init;
> + struct clk_hw *hw;
> + struct intel_clk_mux *mux;
> + u32 reg = list->mux_off;
> + u8 shift = list->mux_shift;
> + u8 width = list->mux_width;
> + unsigned long cflags = list->mux_flags;
> + int ret;
> +
> + mux = kzalloc(sizeof(*mux), GFP_KERNEL);
> + if (!mux)
> + return ERR_PTR(-ENOMEM);
> +
> + init.name = list->name;
> + init.ops = &intel_clk_mux_ops;
> + init.flags = list->flags | CLK_IS_BASIC;
> + init.parent_names = list->parent_names;
> + init.num_parents = list->num_parents;
> +
> + mux->map = ctx->map;
> + mux->reg = reg;
> + mux->shift = shift;
> + mux->width = width;
> + mux->flags = cflags;
> + mux->hw.init = &init;
> +
> + hw = &mux->hw;
> + ret = clk_hw_register(NULL, hw);
> + if (ret) {
> + kfree(mux);
> + return ERR_PTR(ret);
> + }
> +
> + if (cflags & CLOCK_FLAG_VAL_INIT)
> + intel_set_clk_val(ctx->map, reg, shift, width, list->mux_val);
> +
> + return hw->clk;
> +}
> +
> +static unsigned long
> +intel_clk_divider_recalc_rate(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + struct intel_clk_divider *divider = to_intel_clk_divider(hw);
> + unsigned int val;
> +
> + val = intel_get_clk_val(divider->map, divider->reg,
> + divider->shift, divider->width);
> + return divider_recalc_rate(hw, parent_rate, val, divider->table,
> + divider->flags, divider->width);
> +}
> +
> +static long
> +intel_clk_divider_round_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long *prate)
> +{
> + struct intel_clk_divider *divider = to_intel_clk_divider(hw);
> +
> + return divider_round_rate(hw, rate, prate, divider->table,
> + divider->width, divider->flags);
> +}
> +
> +static int
> +intel_clk_divider_set_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long prate)
> +{
> + struct intel_clk_divider *divider = to_intel_clk_divider(hw);
> + int value;
> +
> + value = divider_get_val(rate, prate, divider->table,
> + divider->width, divider->flags);
> + if (value < 0)
> + return value;
> +
> + intel_set_clk_val(divider->map, divider->reg,
> + divider->shift, divider->width, value);
> +
> + return 0;
> +}
> +
> +const static struct clk_ops intel_clk_divider_ops = {
> + .recalc_rate = intel_clk_divider_recalc_rate,
> + .round_rate = intel_clk_divider_round_rate,
> + .set_rate = intel_clk_divider_set_rate,
> +};
> +
> +static struct clk
> +*intel_clk_register_divider(struct intel_clk_provider *ctx,
> + struct intel_clk_branch *list)
> +{
> + struct clk_init_data init;
> + struct clk_hw *hw;
> + struct intel_clk_divider *div;
> + u32 reg = list->div_off;
> + u8 shift = list->div_shift;
> + u8 width = list->div_width;
> + unsigned long cflags = list->div_flags;
> + int ret;
> +
> + div = kzalloc(sizeof(*div), GFP_KERNEL);
> + if (!div)
> + return ERR_PTR(-ENOMEM);
> +
> + init.name = list->name;
> + init.ops = &intel_clk_divider_ops;
> + init.flags = list->flags | CLK_IS_BASIC;
> + init.parent_names = &list->parent_names[0];
> + init.num_parents = 1;
> +
> + div->map = ctx->map;
> + div->reg = reg;
> + div->shift = shift;
> + div->width = width;
> + div->flags = cflags;
> + div->table = list->div_table;
> + div->hw.init = &init;
> +
> + hw = &div->hw;
> + ret = clk_hw_register(NULL, hw);
> + if (ret) {
> + pr_err("%s: register clk: %s failed!\n",
> + __func__, list->name);
> + kfree(div);
> + return ERR_PTR(ret);
> + }
> +
> + if (cflags & CLOCK_FLAG_VAL_INIT)
> + intel_set_clk_val(ctx->map, reg, shift, width, list->div_val);
> +
> + return hw->clk;
> +}
> +
> +static struct clk
> +*intel_clk_register_fixed_factor(struct intel_clk_provider *ctx,
> + struct intel_clk_branch *list)
> +{
> + struct clk_hw *hw;
> +
> + hw = clk_hw_register_fixed_factor(NULL, list->name,
> + list->parent_names[0], list->flags,
> + list->mult, list->div);
> + if (IS_ERR(hw))
> + return ERR_CAST(hw);
> +
> + if (list->div_flags & CLOCK_FLAG_VAL_INIT)
> + intel_set_clk_val(ctx->map, list->div_off, list->div_shift,
> + list->div_width, list->div_val);
> +
> + return hw->clk;
> +}
> +
> +static int
> +intel_clk_gate_enable(struct clk_hw *hw)
> +{
> + struct intel_clk_gate *gate = to_intel_clk_gate(hw);
> + unsigned int reg;
> +
> + if (gate->flags & GATE_CLK_VT) {
> + gate->reg = 1;
> + return 0;
> + }
> +
> + if (gate->flags & GATE_CLK_HW) {
> + reg = GATE_HW_REG_EN(gate->reg);
> + } else if (gate->flags & GATE_CLK_SW) {
> + reg = gate->reg;
> + } else {
> + pr_err("%s: gate clk: %s: flag 0x%lx not supported!\n",
> + __func__, clk_hw_get_name(hw), gate->flags);
> + return 0;
> + }
> +
> + intel_set_clk_val(gate->map, reg, gate->shift, 1, 1);
> +
> + return 0;
> +}
> +
> +static void
> +intel_clk_gate_disable(struct clk_hw *hw)
> +{
> + struct intel_clk_gate *gate = to_intel_clk_gate(hw);
> + unsigned int reg;
> + unsigned int set;
> +
> + if (gate->flags & GATE_CLK_VT) {
> + gate->reg = 0;
> + return;
> + }
> +
> + if (gate->flags & GATE_CLK_HW) {
> + reg = GATE_HW_REG_DIS(gate->reg);
> + set = 1;
> + } else if (gate->flags & GATE_CLK_SW) {
> + reg = gate->reg;
> + set = 0;
> + } else {
> + pr_err("%s: gate clk: %s: flag 0x%lx not supported!\n",
> + __func__, clk_hw_get_name(hw), gate->flags);
> + return;
> + }
> +
> + intel_set_clk_val(gate->map, reg, gate->shift, 1, set);
> +}
> +
> +static int
> +intel_clk_gate_is_enabled(struct clk_hw *hw)
> +{
> + struct intel_clk_gate *gate = to_intel_clk_gate(hw);
> + unsigned int reg;
> +
> + if (gate->flags & GATE_CLK_VT)
> + return gate->reg;
> +
> + if (gate->flags & GATE_CLK_HW) {
> + reg = GATE_HW_REG_STAT(gate->reg);
> + } else if (gate->flags & GATE_CLK_SW) {
> + reg = gate->reg;
> + } else {
> + pr_err("%s: gate clk: %s: flag 0x%lx not supported!\n",
> + __func__, clk_hw_get_name(hw), gate->flags);
> + return 0;
> + }
> +
> + return intel_get_clk_val(gate->map, reg, gate->shift, 1);
> +}
> +
> +const static struct clk_ops intel_clk_gate_ops = {
> + .enable = intel_clk_gate_enable,
> + .disable = intel_clk_gate_disable,
> + .is_enabled = intel_clk_gate_is_enabled,
> +};
> +
> +static struct clk
> +*intel_clk_register_gate(struct intel_clk_provider *ctx,
> + struct intel_clk_branch *list)
> +{
> + struct clk_init_data init;
Please init the init struct with { } so that future possible additions
to the structure don't require us to hunt this silent corruption down
later.
> + struct clk_hw *hw;
> + struct intel_clk_gate *gate;
> + u32 reg = list->gate_off;
> + u8 shift = list->gate_shift;
> + unsigned long cflags = list->gate_flags;
> + const char *pname = list->parent_names[0];
> + int ret;
> +
> + gate = kzalloc(sizeof(*gate), GFP_KERNEL);
> + if (!gate)
> + return ERR_PTR(-ENOMEM);
> +
> + init.name = list->name;
> + init.ops = &intel_clk_gate_ops;
> + init.flags = list->flags | CLK_IS_BASIC;
> + init.parent_names = pname ? &pname : NULL;
> + init.num_parents = pname ? 1 : 0;
> +
> + gate->map = ctx->map;
> + gate->reg = reg;
> + gate->shift = shift;
> + gate->flags = cflags;
> + gate->hw.init = &init;
> +
> + hw = &gate->hw;
> + ret = clk_hw_register(NULL, hw);
> + if (ret) {
> + kfree(gate);
> + return ERR_PTR(ret);
> + }
> +
> + if (cflags & CLOCK_FLAG_VAL_INIT)
> + intel_set_clk_val(ctx->map, reg, shift, 1, list->gate_val);
> +
> + return hw->clk;
> +}
> +
> +void intel_clk_register_branches(struct intel_clk_provider *ctx,
> + struct intel_clk_branch *list,
> + unsigned int nr_clk)
> +{
> + struct clk *clk;
> + unsigned int idx;
> +
> + for (idx = 0; idx < nr_clk; idx++, list++) {
> + switch (list->type) {
> + case intel_clk_fixed:
Please use uppercase for enums.
> + clk = intel_clk_register_fixed(ctx, list);
> + break;
> + case intel_clk_mux:
> + clk = intel_clk_register_mux(ctx, list);
> + break;
> + case intel_clk_divider:
> + clk = intel_clk_register_divider(ctx, list);
> + break;
> + case intel_clk_fixed_factor:
> + clk = intel_clk_register_fixed_factor(ctx, list);
> + break;
> + case intel_clk_gate:
> + clk = intel_clk_register_gate(ctx, list);
> + break;
> + default:
> + pr_err("%s: type: %u not supported!\n",
> + __func__, list->type);
> + return;
> + }
> +
> + if (IS_ERR(clk)) {
> + pr_err("%s: register clk: %s, type: %u failed!\n",
> + __func__, list->name, list->type);
> + return;
> + }
> +
> + intel_clk_add_lookup(ctx, clk, list->id);
> + }
> +}
> +
> +struct intel_clk_provider * __init
> +intel_clk_init(struct device_node *np, struct regmap *map, unsigned int nr_clks)
> +{
> + struct intel_clk_provider *ctx;
> + struct clk **clks;
> +
> + ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
> + if (!ctx)
> + return ERR_PTR(-ENOMEM);
> +
> + clks = kcalloc(nr_clks, sizeof(*clks), GFP_KERNEL);
> + if (!clks) {
> + kfree(ctx);
> + return ERR_PTR(-ENOMEM);
> + }
> +
> + memset_p((void **)clks, ERR_PTR(-ENOENT), nr_clks);
> + ctx->map = map;
> + ctx->clk_data.clks = clks;
> + ctx->clk_data.clk_num = nr_clks;
> + ctx->np = np;
> +
> + return ctx;
> +}
> +
> +void __init intel_clk_register_osc(struct intel_clk_provider *ctx,
> + struct intel_osc_clk *osc,
> + unsigned int nr_clks)
> +{
> + u32 freq;
> + struct clk *clk;
> + int idx;
> +
> + for (idx = 0; idx < nr_clks; idx++, osc++) {
> + if (!osc->dt_freq ||
> + of_property_read_u32(ctx->np, osc->dt_freq, &freq))
> + freq = osc->def_rate;
> +
> + clk = clk_register_fixed_rate(NULL, osc->name, NULL, 0, freq);
Should come from DT itself.
> + if (IS_ERR(clk)) {
> + pr_err("%s: Failed to register clock: %s\n",
> + __func__, osc->name);
> + return;
> + }
> +
> + intel_clk_add_lookup(ctx, clk, osc->id);
> + }
> +}
> diff --git a/drivers/clk/intel/clk-cgu.h b/drivers/clk/intel/clk-cgu.h
> new file mode 100644
> index 000000000000..6dc4e45fc499
> --- /dev/null
> +++ b/drivers/clk/intel/clk-cgu.h
> @@ -0,0 +1,259 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright(c) 2018 Intel Corporation.
> + * Zhu YiXin <Yixin.zhu@intel.com>
> + */
> +
> +#ifndef __INTEL_CLK_H
> +#define __INTEL_CLK_H
> +
> +#define PNAME(x) static const char *const x[] __initconst
> +
> +struct intel_clk_mux {
> + struct clk_hw hw;
> + struct regmap *map;
> + unsigned int reg;
> + u8 shift;
> + u8 width;
> + unsigned long flags;
> +};
> +
> +struct intel_clk_divider {
> + struct clk_hw hw;
> + struct regmap *map;
> + unsigned int reg;
> + u8 shift;
> + u8 width;
> + unsigned long flags;
> + const struct clk_div_table *table;
> +};
> +
> +struct intel_clk_gate {
> + struct clk_hw hw;
> + struct regmap *map;
> + unsigned int reg;
> + u8 shift;
> + unsigned long flags;
> +};
> +
> +enum intel_clk_type {
> + intel_clk_fixed,
> + intel_clk_mux,
> + intel_clk_divider,
> + intel_clk_fixed_factor,
> + intel_clk_gate,
> +};
> +
> +/**
> + * struct intel_clk_provider
> + * @map: regmap type base address for register.
> + * @np: device node
> + * @clk_data: array of hw clocks and clk number.
> + */
> +struct intel_clk_provider {
> + struct regmap *map;
> + struct device_node *np;
> + struct clk_onecell_data clk_data;
Please register clk_hw pointers instead of clk pointers with the of
provider APIs.
> +};
> +
> +/**
> + * struct intel_pll_clk
> + * @id: plaform specific id of the clock.
> + * @name: name of this pll clock.
> + * @parent_names: name of the parent clock.
> + * @num_parents: number of parents.
> + * @flags: optional flags for basic clock.
> + * @type: platform type of pll.
> + * @reg: offset of the register.
> + * @mult: init value of mulitplier.
> + * @div: init value of divider.
> + * @frac: init value of fraction.
> + * @rate_table: table of pll clock rate.
Please drop the full-stop on kernel doc one-liners like this.
> + */
> +struct intel_pll_clk {
> + unsigned int id;
> + const char *name;
> + const char *const *parent_names;
> + u8 num_parents;
Can the PLL have multiple parents?
> + unsigned long flags;
> + enum intel_pll_type type;
> + int reg;
> + unsigned int mult;
> + unsigned int div;
> + unsigned int frac;
> + const struct intel_pll_rate_table *rate_table;
> +};
> +
> +#define INTEL_PLL(_id, _type, _name, _pnames, _flags, \
> + _reg, _rtable, _mult, _div, _frac) \
> + { \
> + .id = _id, \
> + .type = _type, \
> + .name = _name, \
> + .parent_names = _pnames, \
> + .num_parents = ARRAY_SIZE(_pnames), \
> + .flags = _flags, \
> + .reg = _reg, \
> + .rate_table = _rtable, \
> + .mult = _mult, \
> + .div = _div, \
> + .frac = _frac \
> + }
> +
> +/**
> + * struct intel_osc_clk
> + * @id: platform specific id of the clock.
> + * @name: name of the osc clock.
> + * @dt_freq: frequency node name in device tree.
> + * @def_rate: default rate of the osc clock.
> + * @flags: optional flags for basic clock.
There aren't flags though. I'm very confused by this kernel-doc too.
Looks like something that should be done with a fixed rate clk in DT.
> + */
> +struct intel_osc_clk {
> + unsigned int id;
> + const char *name;
> + const char *dt_freq;
> + const u32 def_rate;
> +};
> +
> +#define INTEL_OSC(_id, _name, _freq, _rate) \
> + { \
> + .id = _id, \
> + .name = _name, \
> + .dt_freq = _freq, \
> + .def_rate = _rate, \
> + }
> +
> +struct intel_clk_branch {
Seems to be more like intel_clk instead of intel_clk_branch because it
does lots of stuff.
> + unsigned int id;
> + enum intel_clk_type type;
> + const char *name;
> + const char *const *parent_names;
> + u8 num_parents;
> + unsigned long flags;
> + unsigned int mux_off;
> + u8 mux_shift;
> + u8 mux_width;
> + unsigned long mux_flags;
> + unsigned int mux_val;
> + unsigned int div_off;
> + u8 div_shift;
> + u8 div_width;
> + unsigned long div_flags;
> + unsigned int div_val;
> + const struct clk_div_table *div_table;
> + unsigned int gate_off;
> + u8 gate_shift;
> + unsigned long gate_flags;
> + unsigned int gate_val;
> + unsigned int mult;
> + unsigned int div;
> +};
> +
> +/* clock flags definition */
> +#define CLOCK_FLAG_VAL_INIT BIT(16)
> +#define GATE_CLK_HW BIT(17)
> +#define GATE_CLK_SW BIT(18)
> +#define GATE_CLK_VT BIT(19)
What does VT mean? Virtual?
> +
> +#define INTEL_MUX(_id, _name, _pname, _f, _reg, \
> + _shift, _width, _cf, _v) \
> + { \
> + .id = _id, \
> + .type = intel_clk_mux, \
> + .name = _name, \
> + .parent_names = _pname, \
> + .num_parents = ARRAY_SIZE(_pname), \
> + .flags = _f, \
> + .mux_off = _reg, \
> + .mux_shift = _shift, \
> + .mux_width = _width, \
> + .mux_flags = _cf, \
> + .mux_val = _v, \
> + }
> +
> +#define INTEL_DIV(_id, _name, _pname, _f, _reg, \
> + _shift, _width, _cf, _v, _dtable) \
> + { \
> + .id = _id, \
> + .type = intel_clk_divider, \
> + .name = _name, \
> + .parent_names = (const char *[]) { _pname }, \
> + .num_parents = 1, \
> + .flags = _f, \
> + .div_off = _reg, \
> + .div_shift = _shift, \
> + .div_width = _width, \
> + .div_flags = _cf, \
> + .div_val = _v, \
> + .div_table = _dtable, \
> + }
> +
> +#define INTEL_GATE(_id, _name, _pname, _f, _reg, \
> + _shift, _cf, _v) \
> + { \
> + .id = _id, \
> + .type = intel_clk_gate, \
> + .name = _name, \
> + .parent_names = (const char *[]) { _pname }, \
> + .num_parents = !_pname ? 0 : 1, \
> + .flags = _f, \
> + .gate_off = _reg, \
> + .gate_shift = _shift, \
> + .gate_flags = _cf, \
> + .gate_val = _v, \
> + }
> +
> +#define INTEL_FIXED(_id, _name, _pname, _f, _reg, \
> + _shift, _width, _cf, _freq, _v) \
> + { \
> + .id = _id, \
> + .type = intel_clk_fixed, \
> + .name = _name, \
> + .parent_names = (const char *[]) { _pname }, \
> + .num_parents = !_pname ? 0 : 1, \
> + .flags = _f, \
> + .div_off = _reg, \
> + .div_shift = _shift, \
> + .div_width = _width, \
> + .div_flags = _cf, \
> + .div_val = _v, \
> + .mux_flags = _freq, \
> + }
> +
> +#define INTEL_FIXED_FACTOR(_id, _name, _pname, _f, _reg, \
> + _shift, _width, _cf, _v, _m, _d) \
> + { \
> + .id = _id, \
> + .type = intel_clk_fixed_factor, \
> + .name = _name, \
> + .parent_names = (const char *[]) { _pname }, \
> + .num_parents = 1, \
> + .flags = _f, \
> + .div_off = _reg, \
> + .div_shift = _shift, \
> + .div_width = _width, \
> + .div_flags = _cf, \
> + .div_val = _v, \
> + .mult = _m, \
> + .div = _d, \
> + }
> +
> +void intel_set_clk_val(struct regmap *map, u32 reg, u8 shift,
> + u8 width, u32 set_val);
> +u32 intel_get_clk_val(struct regmap *map, u32 reg, u8 shift, u8 width);
> +void intel_clk_add_lookup(struct intel_clk_provider *ctx,
> + struct clk *clk, unsigned int id);
> +void __init intel_clk_of_add_provider(struct device_node *np,
> + struct intel_clk_provider *ctx);
> +struct intel_clk_provider * __init
> +intel_clk_init(struct device_node *np, struct regmap *map,
> + unsigned int nr_clks);
> +void __init intel_clk_register_osc(struct intel_clk_provider *ctx,
> + struct intel_osc_clk *osc,
> + unsigned int nr_clks);
Remove __init from headers files. It does nothing.
> +void intel_clk_register_branches(struct intel_clk_provider *ctx,
> + struct intel_clk_branch *list,
> + unsigned int nr_clk);
> +void intel_clk_register_plls(struct intel_clk_provider *ctx,
> + struct intel_pll_clk *list, unsigned int nr_clk);
> +#endif /* __INTEL_CLK_H */
> diff --git a/drivers/clk/intel/clk-grx500.c b/drivers/clk/intel/clk-grx500.c
> new file mode 100644
> index 000000000000..5c2546f82579
> --- /dev/null
> +++ b/drivers/clk/intel/clk-grx500.c
> @@ -0,0 +1,168 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2018 Intel Corporation.
> + * Zhu YiXin <Yixin.zhu@intel.com>
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/regmap.h>
> +#include <linux/spinlock.h>
Used?
> +#include <dt-bindings/clock/intel,grx500-clk.h>
> +
> +#include "clk-cgu-pll.h"
> +#include "clk-cgu.h"
> +
> +#define PLL_DIV_WIDTH 4
> +
> +/* Gate1 clock shift */
> +#define G_VCODEC_SHIFT 2
> +#define G_DMA0_SHIFT 5
> +#define G_USB0_SHIFT 6
> +#define G_SPI1_SHIFT 7
> +#define G_SPI0_SHIFT 8
> +#define G_CBM_SHIFT 9
> +#define G_EBU_SHIFT 10
> +#define G_SSO_SHIFT 11
> +#define G_GPTC0_SHIFT 12
> +#define G_GPTC1_SHIFT 13
> +#define G_GPTC2_SHIFT 14
> +#define G_UART_SHIFT 17
> +#define G_CPYTO_SHIFT 20
> +#define G_SECPT_SHIFT 21
> +#define G_TOE_SHIFT 22
> +#define G_MPE_SHIFT 23
> +#define G_TDM_SHIFT 25
> +#define G_PAE_SHIFT 26
> +#define G_USB1_SHIFT 27
> +#define G_SWITCH_SHIFT 28
> +
> +/* Gate2 clock shift */
> +#define G_PCIE0_SHIFT 1
> +#define G_PCIE1_SHIFT 17
> +#define G_PCIE2_SHIFT 25
> +
> +/* Register definition */
> +#define GRX500_PLL0A_CFG0 0x0004
> +#define GRX500_PLL0A_CFG1 0x0008
> +#define GRX500_PLL0B_CFG0 0x0034
> +#define GRX500_PLL0B_CFG1 0x0038
> +#define GRX500_LCPLL_CFG0 0x0094
> +#define GRX500_LCPLL_CFG1 0x0098
> +#define GRX500_IF_CLK 0x00c4
> +#define GRX500_CLK_GSR1 0x0120
> +#define GRX500_CLK_GSR2 0x0130
> +
> +static const struct clk_div_table pll_div[] = {
> + {1, 2},
Please write it like
{ 1, 2 },
instead.
> + {2, 3},
> + {3, 4},
> + {4, 5},
> + {5, 6},
> + {6, 8},
> + {7, 10},
> + {8, 12},
> + {9, 16},
> + {10, 20},
> + {11, 24},
> + {12, 32},
> + {13, 40},
> + {14, 48},
> + {15, 64}
> +};
> +
> +enum grx500_plls {
> + pll0a, pll0b, pll3,
> +};
What's the point of the enum?
> +
> +PNAME(pll_p) = { "osc" };
> +PNAME(cpu_p) = { "cpu0", "cpu1" };
> +
> +static struct intel_osc_clk grx500_osc_clks[] __initdata = {
> + INTEL_OSC(CLK_OSC, "osc", "intel,osc-frequency", 40000000),
> +};
> +
> +static struct intel_pll_clk grx500_pll_clks[] __initdata = {
> + [pll0a] = INTEL_PLL(CLK_PLL0A, pll_grx500, "pll0a",
> + pll_p, 0, GRX500_PLL0A_CFG0, NULL, 0, 0, 0),
> + [pll0b] = INTEL_PLL(CLK_PLL0B, pll_grx500, "pll0b",
> + pll_p, 0, GRX500_PLL0B_CFG0, NULL, 0, 0, 0),
> + [pll3] = INTEL_PLL(CLK_PLL3, pll_grx500, "pll3",
> + pll_p, 0, GRX500_LCPLL_CFG0, NULL, 0, 0, 0),
> +};
> +
> +static struct intel_clk_branch grx500_branch_clks[] __initdata = {
> + INTEL_DIV(CLK_CBM, "cbm", "pll0a", 0, GRX500_PLL0A_CFG1,
> + 0, PLL_DIV_WIDTH, 0, 0, pll_div),
> + INTEL_DIV(CLK_NGI, "ngi", "pll0a", 0, GRX500_PLL0A_CFG1,
> + 4, PLL_DIV_WIDTH, 0, 0, pll_div),
> + INTEL_DIV(CLK_SSX4, "ssx4", "pll0a", 0, GRX500_PLL0A_CFG1,
> + 8, PLL_DIV_WIDTH, 0, 0, pll_div),
> + INTEL_DIV(CLK_CPU0, "cpu0", "pll0a", 0, GRX500_PLL0A_CFG1,
> + 12, PLL_DIV_WIDTH, 0, 0, pll_div),
> + INTEL_DIV(CLK_PAE, "pae", "pll0b", 0, GRX500_PLL0B_CFG1,
> + 0, PLL_DIV_WIDTH, 0, 0, pll_div),
> + INTEL_DIV(CLK_GSWIP, "gswip", "pll0b", 0, GRX500_PLL0B_CFG1,
> + 4, PLL_DIV_WIDTH, 0, 0, pll_div),
> + INTEL_DIV(CLK_DDR, "ddr", "pll0b", 0, GRX500_PLL0B_CFG1,
> + 8, PLL_DIV_WIDTH, 0, 0, pll_div),
> + INTEL_DIV(CLK_CPU1, "cpu1", "pll0b", 0, GRX500_PLL0B_CFG1,
> + 12, PLL_DIV_WIDTH, 0, 0, pll_div),
> + INTEL_MUX(CLK_CPU, "cpu", cpu_p, CLK_SET_RATE_PARENT,
> + GRX500_PLL0A_CFG1, 29, 1, 0, 0),
> + INTEL_GATE(GCLK_DMA0, "g_dma0", NULL, 0, GRX500_CLK_GSR1,
> + G_DMA0_SHIFT, GATE_CLK_HW, 0),
> + INTEL_GATE(GCLK_USB0, "g_usb0", NULL, 0, GRX500_CLK_GSR1,
> + G_USB0_SHIFT, GATE_CLK_HW, 0),
> + INTEL_GATE(GCLK_GPTC0, "g_gptc0", NULL, 0, GRX500_CLK_GSR1,
> + G_GPTC0_SHIFT, GATE_CLK_HW, 0),
> + INTEL_GATE(GCLK_GPTC1, "g_gptc1", NULL, 0, GRX500_CLK_GSR1,
> + G_GPTC1_SHIFT, GATE_CLK_HW, 0),
> + INTEL_GATE(GCLK_GPTC2, "g_gptc2", NULL, 0, GRX500_CLK_GSR1,
> + G_GPTC2_SHIFT, GATE_CLK_HW, 0),
> + INTEL_GATE(GCLK_UART, "g_uart", NULL, 0, GRX500_CLK_GSR1,
> + G_UART_SHIFT, GATE_CLK_HW, 0),
> + INTEL_GATE(GCLK_PCIE0, "g_pcie0", NULL, 0, GRX500_CLK_GSR2,
> + G_PCIE0_SHIFT, GATE_CLK_HW, 0),
> + INTEL_GATE(GCLK_PCIE1, "g_pcie1", NULL, 0, GRX500_CLK_GSR2,
> + G_PCIE1_SHIFT, GATE_CLK_HW, 0),
> + INTEL_GATE(GCLK_PCIE2, "g_pcie2", NULL, 0, GRX500_CLK_GSR2,
> + G_PCIE2_SHIFT, GATE_CLK_HW, 0),
> + INTEL_GATE(GCLK_I2C, "g_i2c", NULL, 0, 0, 0, GATE_CLK_VT, 0),
> + INTEL_FIXED(CLK_VOICE, "voice", NULL, 0, GRX500_IF_CLK, 14, 2,
> + CLOCK_FLAG_VAL_INIT, 8192000, 2),
> + INTEL_FIXED_FACTOR(CLK_DDRPHY, "ddrphy", "ddr", 0, 0, 0,
> + 0, 0, 0, 2, 1),
> + INTEL_FIXED_FACTOR(CLK_PCIE, "pcie", "pll3", 0, 0, 0,
> + 0, 0, 0, 1, 40),
> +};
> +
> +static void __init grx500_clk_init(struct device_node *np)
> +{
> + struct intel_clk_provider *ctx;
> + struct regmap *map;
> +
> + map = syscon_node_to_regmap(np);
> + if (IS_ERR(map))
> + return;
> +
> + ctx = intel_clk_init(np, map, CLK_NR_CLKS);
> + if (IS_ERR(ctx)) {
> + regmap_exit(map);
> + return;
> + }
> +
> + intel_clk_register_osc(ctx, grx500_osc_clks,
> + ARRAY_SIZE(grx500_osc_clks));
> + intel_clk_register_plls(ctx, grx500_pll_clks,
> + ARRAY_SIZE(grx500_pll_clks));
> + intel_clk_register_branches(ctx, grx500_branch_clks,
> + ARRAY_SIZE(grx500_branch_clks));
> + of_clk_add_provider(np, of_clk_src_onecell_get, &ctx->clk_data);
> +
> + pr_debug("%s clk init done!\n", __func__);
Yay!!!
> +}
> +
> +CLK_OF_DECLARE(intel_grx500_cgu, "intel,grx500-cgu", grx500_clk_init);
Any reason a platform driver can't be used instead of CLK_OF_DECLARE()?
^ permalink raw reply
* Re: [PATCH v2 08/18] serial: intel: Get serial id from dts
From: Wu, Songjun @ 2018-08-08 4:05 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: hua.ma, yixin.zhu, chuanhua.lei, qi-ming.wu,
Linux MIPS Mailing List, linux-clk, open list:SERIAL DRIVERS,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Greg KH, Linux Kernel Mailing List, Jiri Slaby
In-Reply-To: <CAMuHMdXkGchPN337dXbBVOFsb1o-Tkh8S_z=uCm3Z0sDjPVMKA@mail.gmail.com>
On 8/7/2018 3:33 PM, Geert Uytterhoeven wrote:
> Hi Songjun,
>
> On Fri, Aug 3, 2018 at 5:04 AM Songjun Wu <songjun.wu@linux.intel.com> wrote:
>> Get serial id from dts.
>>
>> "#ifdef CONFIG_LANTIQ" preprocessor is used because LTQ_EARLY_ASC
>> macro is defined in lantiq_soc.h.
>> lantiq_soc.h is in arch path for legacy product support.
>>
>> arch/mips/include/asm/mach-lantiq/xway/lantiq_soc.h
>>
>> If "#ifdef preprocessor" is changed to
>> "if (IS_ENABLED(CONFIG_LANTIQ))", when CONFIG_LANTIQ is not enabled,
>> code using LTQ_EARLY_ASC is compiled.
>> Compilation will fail for no LTQ_EARLY_ASC defined.
>>
>> Signed-off-by: Songjun Wu <songjun.wu@linux.intel.com>
> Thanks for your patch!
>
>> @@ -699,9 +700,19 @@ lqasc_probe(struct platform_device *pdev)
>> return -ENODEV;
>> }
>>
>> - /* check if this is the console port */
>> - if (mmres->start != CPHYSADDR(LTQ_EARLY_ASC))
>> - line = 1;
>> + /* get serial id */
>> + line = of_alias_get_id(node, "serial");
>> + if (line < 0) {
>> +#ifdef CONFIG_LANTIQ
>> + if (mmres->start == CPHYSADDR(LTQ_EARLY_ASC))
>> + line = 0;
>> + else
>> + line = 1;
>> +#else
>> + dev_err(&pdev->dev, "failed to get alias id, errno %d\n", line);
>> + return line;
> Please note that not providing a fallback here makes life harder when using
> DT overlays.
> See the description of commit 7678f4c20fa7670f ("serial: sh-sci: Add support
> for dynamic instances") for background info.
Thanks for your comment.
The logic in commit 7678f4c20fa7670f is not suitable here.
We need to know which serial instance is used for console.
We cannot use dynamic serial instance here.
^ permalink raw reply
* Re: [PATCH v2 03/18] dt-bindings: clk: Add documentation of grx500 clock controller
From: yixin zhu @ 2018-08-08 3:08 UTC (permalink / raw)
To: Rob Herring, Songjun Wu
Cc: hua.ma, chuanhua.lei, qi-ming.wu, Linux-MIPS, linux-clk,
open list:SERIAL DRIVERS, devicetree, Michael Turquette,
Stephen Boyd, linux-kernel@vger.kernel.org, Mark Rutland
In-Reply-To: <CAL_JsqKLa1e3X+ddAoVdEwBQFd6tsL=RjOLcTLdfoc6mpKuefQ@mail.gmail.com>
On 8/6/2018 11:18 PM, Rob Herring wrote:
> On Thu, Aug 2, 2018 at 9:03 PM Songjun Wu <songjun.wu@linux.intel.com> wrote:
>> From: Yixin Zhu <yixin.zhu@linux.intel.com>
>>
>> This patch adds binding documentation for grx500 clock controller.
>>
>> Signed-off-by: YiXin Zhu <yixin.zhu@linux.intel.com>
>> Signed-off-by: Songjun Wu <songjun.wu@linux.intel.com>
>> ---
>>
>> Changes in v2:
>> - Rewrite clock driver's dt-binding document according to Rob Herring's
>> comments.
>> - Simplify device tree docoment, remove some clock description.
>>
>> .../devicetree/bindings/clock/intel,grx500-clk.txt | 39 ++++++++++++++++++++++
> Please match the compatible string: intel,grx500-cgu.txt
Will update to use same name.
>
>> 1 file changed, 39 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/clock/intel,grx500-clk.txt
>>
>> diff --git a/Documentation/devicetree/bindings/clock/intel,grx500-clk.txt b/Documentation/devicetree/bindings/clock/intel,grx500-clk.txt
>> new file mode 100644
>> index 000000000000..e54e1dad9196
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/intel,grx500-clk.txt
>> @@ -0,0 +1,39 @@
>> +Device Tree Clock bindings for grx500 PLL controller.
>> +
>> +This binding uses the common clock binding:
>> + Documentation/devicetree/bindings/clock/clock-bindings.txt
>> +
>> +The grx500 clock controller supplies clock to various controllers within the
>> +SoC.
>> +
>> +Required properties for clock node
>> +- compatible: Should be "intel,grx500-cgu".
>> +- reg: physical base address of the controller and length of memory range.
>> +- #clock-cells: should be 1.
>> +
>> +Optional Propteries:
>> +- intel,osc-frequency: frequency of the osc clock.
>> +if missing, driver will use clock rate defined in the driver.
> This should use a fixed-clock node instead.
Yes, This is a fixed clock node registered in driver code.
The frequency of the fixed clock is designed to be overwritten by device
tree in case some one verify
clock driver in the emulation platform or in some cases frequency other
than driver defined one is preferred.
These kinds of cases are very rare. But I feel it would be better to
have a way to use customized frequency.
The frequency defined in device tree will overwritten driver defined
frequency before registering fixed-clock node.
>> +
>> +Example: Clock controller node:
>> +
>> + cgu: cgu@16200000 {
>> + compatible = "intel,grx500-cgu", "syscon";
>> + reg = <0x16200000 0x200>;
>> + #clock-cells = <1>;
>> + };
>> +
>> +
>> +Example: UART controller node that consumes the clock generated by clock
>> + controller.
>> +
>> + asc0: serial@16600000 {
>> + compatible = "lantiq,asc";
>> + reg = <0x16600000 0x100000>;
>> + interrupt-parent = <&gic>;
>> + interrupts = <GIC_SHARED 103 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SHARED 105 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SHARED 106 IRQ_TYPE_LEVEL_HIGH>;
>> + clocks = <&cgu CLK_SSX4>, <&cgu GCLK_UART>;
>> + clock-names = "freq", "asc";
>> + };
>> --
>> 2.11.0
>>
^ permalink raw reply
* Re: [PATCH v2 02/18] clk: intel: Add clock driver for Intel MIPS SoCs
From: yixin zhu @ 2018-08-08 2:51 UTC (permalink / raw)
To: Rob Herring, Songjun Wu
Cc: hua.ma, chuanhua.lei, qi-ming.wu, Linux-MIPS, linux-clk,
open list:SERIAL DRIVERS, devicetree, Michael Turquette,
Stephen Boyd, linux-kernel@vger.kernel.org, Mark Rutland
In-Reply-To: <CAL_JsqK5pFNKyAhTTmNpaSnKa_beY3kS8FGtYim8oTgw6oO9Rw@mail.gmail.com>
On 8/6/2018 11:19 PM, Rob Herring wrote:
> On Thu, Aug 2, 2018 at 9:03 PM Songjun Wu <songjun.wu@linux.intel.com> wrote:
>> From: Yixin Zhu <yixin.zhu@linux.intel.com>
>>
>> This driver provides PLL clock registration as well as various clock
>> branches, e.g. MUX clock, gate clock, divider clock and so on.
>>
>> PLLs that provide clock to DDR, CPU and peripherals are shown below:
>>
>> +---------+
>> |--->| LCPLL3 0|--PCIe clk-->
>> XO | +---------+
>> +-----------|
>> | +---------+
>> | | 3|--PAE clk-->
>> |--->| PLL0B 2|--GSWIP clk-->
>> | | 1|--DDR clk-->DDR PHY clk-->
>> | | 0|--CPU1 clk--+ +-----+
>> | +---------+ |--->0 |
>> | | MUX |--CPU clk-->
>> | +---------+ |--->1 |
>> | | 0|--CPU0 clk--+ +-----+
>> |--->| PLLOA 1|--SSX4 clk-->
>> | 2|--NGI clk-->
>> | 3|--CBM clk-->
>> +---------+
>>
>> Signed-off-by: Yixin Zhu <yixin.zhu@linux.intel.com>
>> Signed-off-by: Songjun Wu <songjun.wu@linux.intel.com>
>> ---
>>
>> Changes in v2:
>> - Rewrite clock driver, add platform clock description details in
>> clock driver.
>>
>> drivers/clk/Kconfig | 1 +
>> drivers/clk/Makefile | 3 +
>> drivers/clk/intel/Kconfig | 20 ++
>> drivers/clk/intel/Makefile | 7 +
>> drivers/clk/intel/clk-cgu-pll.c | 166 ++++++++++
>> drivers/clk/intel/clk-cgu-pll.h | 34 ++
>> drivers/clk/intel/clk-cgu.c | 470 +++++++++++++++++++++++++++
>> drivers/clk/intel/clk-cgu.h | 259 +++++++++++++++
>> drivers/clk/intel/clk-grx500.c | 168 ++++++++++
>> include/dt-bindings/clock/intel,grx500-clk.h | 69 ++++
> This belongs with the clk binding patch.
>
> Rob
Thanks for review.
Will move it to clock binding patch.
>
^ permalink raw reply
* Re: [PATCH v3 1/2] tty/serial_core: add ISO7816 infrastructure
From: Christoph Hellwig @ 2018-08-07 13:32 UTC (permalink / raw)
To: Ludovic Desroches
Cc: linux-serial, linux-arch, linux-arm-kernel, gregkh, jslaby, arnd,
richard.genoud, nicolas.ferre, alexandre.belloni, linux-kernel
In-Reply-To: <20180807130049.5957-2-ludovic.desroches@microchip.com>
On Tue, Aug 07, 2018 at 03:00:48PM +0200, Ludovic Desroches wrote:
> From: Nicolas Ferre <nicolas.ferre@microchip.com>
>
> Add the ISO7816 ioctl and associated accessors and data structure.
> Drivers can then use this common implementation to handle ISO7816.
It would help a lot of you'd explain what ISO7816 is. Without that
the changelog is rather meaningless.
^ permalink raw reply
* [PATCH v3 2/2] tty/serial: atmel: add ISO7816 support
From: Ludovic Desroches @ 2018-08-07 13:00 UTC (permalink / raw)
To: linux-serial, linux-arch, linux-arm-kernel
Cc: gregkh, jslaby, arnd, richard.genoud, nicolas.ferre,
alexandre.belloni, linux-kernel, Ludovic Desroches
In-Reply-To: <20180807130049.5957-1-ludovic.desroches@microchip.com>
From: Nicolas Ferre <nicolas.ferre@microchip.com>
When mode is set in atmel_config_iso7816() we backup last RS232 mode
for coming back to this mode if requested.
Also allow setup of T=0 and T=1 parameter and basic support in set_termios
function as well.
Signed-off-by: Nicolas Ferre <nicolas.ferre@microchip.com>
[ludovic.desroches@microchip.com: rebase, add check on fidi ratio, checkpatch fixes]
Signed-off-by: Ludovic Desroches <ludovic.desroches@microchip.com>
---
drivers/tty/serial/atmel_serial.c | 211 +++++++++++++++++++++++++++++++++++---
drivers/tty/serial/atmel_serial.h | 6 +-
2 files changed, 201 insertions(+), 16 deletions(-)
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 8e4428725848..4a7ec44b0ace 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -34,6 +34,7 @@
#include <linux/suspend.h>
#include <linux/mm.h>
+#include <asm/div64.h>
#include <asm/io.h>
#include <asm/ioctls.h>
@@ -83,6 +84,11 @@ static void atmel_stop_rx(struct uart_port *port);
#define ATMEL_ISR_PASS_LIMIT 256
+struct atmel_uart_pdata {
+ unsigned int fidi_min;
+ unsigned int fidi_max;
+};
+
struct atmel_dma_buffer {
unsigned char *buf;
dma_addr_t dma_addr;
@@ -114,6 +120,7 @@ struct atmel_uart_char {
*/
struct atmel_uart_port {
struct uart_port uart; /* uart */
+ const struct atmel_uart_pdata *pdata; /* SoC specific parameters */
struct clk *clk; /* uart clock */
int may_wakeup; /* cached value of device_may_wakeup for times we need to disable it */
u32 backup_imr; /* IMR saved during suspend */
@@ -147,6 +154,8 @@ struct atmel_uart_port {
struct circ_buf rx_ring;
struct mctrl_gpios *gpios;
+ u32 backup_mode; /* MR saved during iso7816 operations */
+ u32 backup_brgr; /* BRGR saved during iso7816 operations */
unsigned int tx_done_mask;
u32 fifo_size;
u32 rts_high;
@@ -192,10 +201,34 @@ static struct console atmel_console;
#endif
#if defined(CONFIG_OF)
+static struct atmel_uart_pdata at91rm9200_pdata = {
+ .fidi_min = 1,
+ .fidi_max = 2047,
+};
+
+static struct atmel_uart_pdata at91sam9260_pdata = {
+ .fidi_min = 1,
+ .fidi_max = 2047,
+};
+
+static struct atmel_uart_pdata sama5d3_pdata = {
+ .fidi_min = 3,
+ .fidi_max = 65535,
+};
+
static const struct of_device_id atmel_serial_dt_ids[] = {
- { .compatible = "atmel,at91rm9200-usart" },
- { .compatible = "atmel,at91sam9260-usart" },
- { /* sentinel */ }
+ {
+ .compatible = "atmel,at91rm9200-usart",
+ .data = &at91rm9200_pdata,
+ }, {
+ .compatible = "atmel,at91sam9260-usart",
+ .data = &at91sam9260_pdata,
+ }, {
+ .compatible = "atmel,sama5d3-usart",
+ .data = &sama5d3_pdata,
+ }, {
+ /* sentinel */
+ }
};
#endif
@@ -362,6 +395,127 @@ static int atmel_config_rs485(struct uart_port *port,
return 0;
}
+static unsigned int atmel_calc_cd(struct uart_port *port,
+ struct serial_iso7816 *iso7816conf)
+{
+ struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
+ unsigned int cd;
+ u64 mck_rate;
+
+ mck_rate = (u64)clk_get_rate(atmel_port->clk);
+ do_div(mck_rate, iso7816conf->clk);
+ cd = mck_rate;
+ return cd;
+}
+
+static unsigned int atmel_calc_fidi(struct uart_port *port,
+ struct serial_iso7816 *iso7816conf)
+{
+ u64 fidi = 0;
+
+ if (iso7816conf->sc_fi && iso7816conf->sc_di) {
+ fidi = (u64)iso7816conf->sc_fi;
+ do_div(fidi, iso7816conf->sc_di);
+ }
+ return (u32)fidi;
+}
+
+/* Enable or disable the iso7816 support */
+/* Called with interrupts disabled */
+static int atmel_config_iso7816(struct uart_port *port,
+ struct serial_iso7816 *iso7816conf)
+{
+ struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
+ unsigned int mode;
+ unsigned int cd, fidi;
+ int ret = 0;
+
+ /* Disable interrupts */
+ atmel_uart_writel(port, ATMEL_US_IDR, atmel_port->tx_done_mask);
+
+ mode = atmel_uart_readl(port, ATMEL_US_MR);
+
+ if (iso7816conf->flags & SER_ISO7816_ENABLED) {
+ mode &= ~ATMEL_US_USMODE;
+
+ if (iso7816conf->tg > 255) {
+ dev_err(port->dev, "ISO7816: Timeguard exceeding 255\n");
+ memset(iso7816conf, 0, sizeof(struct serial_iso7816));
+ ret = -EINVAL;
+ goto err_out;
+ }
+
+ if ((iso7816conf->flags & SER_ISO7816_T_PARAM)
+ == SER_ISO7816_T(0)) {
+ mode |= ATMEL_US_USMODE_ISO7816_T0 | ATMEL_US_DSNACK;
+ } else if ((iso7816conf->flags & SER_ISO7816_T_PARAM)
+ == SER_ISO7816_T(1)) {
+ mode |= ATMEL_US_USMODE_ISO7816_T1 | ATMEL_US_INACK;
+ } else {
+ dev_err(port->dev, "ISO7816: Type not supported\n");
+ memset(iso7816conf, 0, sizeof(struct serial_iso7816));
+ ret = -EINVAL;
+ goto err_out;
+ }
+
+ mode &= ~(ATMEL_US_USCLKS | ATMEL_US_NBSTOP | ATMEL_US_PAR);
+
+ /* select mck clock, and output */
+ mode |= ATMEL_US_USCLKS_MCK | ATMEL_US_CLKO;
+ /* set parity for normal/inverse mode + max iterations */
+ mode |= ATMEL_US_PAR_EVEN | ATMEL_US_NBSTOP_1 | ATMEL_US_MAX_ITER(3);
+
+ cd = atmel_calc_cd(port, iso7816conf);
+ fidi = atmel_calc_fidi(port, iso7816conf);
+ if (fidi == 0) {
+ dev_warn(port->dev, "ISO7816 fidi = 0, Generator generates no signal\n");
+ } else if (fidi < atmel_port->pdata->fidi_min
+ || fidi > atmel_port->pdata->fidi_max) {
+ dev_err(port->dev, "ISO7816 fidi = %u, value not supported\n", fidi);
+ memset(iso7816conf, 0, sizeof(struct serial_iso7816));
+ ret = -EINVAL;
+ goto err_out;
+ }
+
+ if (!(port->iso7816.flags & SER_ISO7816_ENABLED)) {
+ /* port not yet in iso7816 mode: store configuration */
+ atmel_port->backup_mode = atmel_uart_readl(port, ATMEL_US_MR);
+ atmel_port->backup_brgr = atmel_uart_readl(port, ATMEL_US_BRGR);
+ }
+
+ atmel_uart_writel(port, ATMEL_US_TTGR, iso7816conf->tg);
+ atmel_uart_writel(port, ATMEL_US_BRGR, cd);
+ atmel_uart_writel(port, ATMEL_US_FIDIR, fidi);
+
+ atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXDIS | ATMEL_US_RXEN);
+ atmel_port->tx_done_mask = ATMEL_US_TXEMPTY | ATMEL_US_NACK | ATMEL_US_ITERATION;
+ } else {
+ dev_dbg(port->dev, "Setting UART back to RS232\n");
+ /* back to last RS232 settings */
+ mode = atmel_port->backup_mode;
+ memset(iso7816conf, 0, sizeof(struct serial_iso7816));
+ atmel_uart_writel(port, ATMEL_US_TTGR, 0);
+ atmel_uart_writel(port, ATMEL_US_BRGR, atmel_port->backup_brgr);
+ atmel_uart_writel(port, ATMEL_US_FIDIR, 0x174);
+
+ if (atmel_use_pdc_tx(port))
+ atmel_port->tx_done_mask = ATMEL_US_ENDTX |
+ ATMEL_US_TXBUFE;
+ else
+ atmel_port->tx_done_mask = ATMEL_US_TXRDY;
+ }
+
+ port->iso7816 = *iso7816conf;
+
+ atmel_uart_writel(port, ATMEL_US_MR, mode);
+
+err_out:
+ /* Enable interrupts */
+ atmel_uart_writel(port, ATMEL_US_IER, atmel_port->tx_done_mask);
+
+ return ret;
+}
+
/*
* Return TIOCSER_TEMT when transmitter FIFO and Shift register is empty.
*/
@@ -481,8 +635,9 @@ static void atmel_stop_tx(struct uart_port *port)
/* Disable interrupts */
atmel_uart_writel(port, ATMEL_US_IDR, atmel_port->tx_done_mask);
- if ((port->rs485.flags & SER_RS485_ENABLED) &&
- !(port->rs485.flags & SER_RS485_RX_DURING_TX))
+ if (((port->rs485.flags & SER_RS485_ENABLED) &&
+ !(port->rs485.flags & SER_RS485_RX_DURING_TX)) ||
+ port->iso7816.flags & SER_ISO7816_ENABLED)
atmel_start_rx(port);
}
@@ -500,8 +655,9 @@ static void atmel_start_tx(struct uart_port *port)
return;
if (atmel_use_pdc_tx(port) || atmel_use_dma_tx(port))
- if ((port->rs485.flags & SER_RS485_ENABLED) &&
- !(port->rs485.flags & SER_RS485_RX_DURING_TX))
+ if (((port->rs485.flags & SER_RS485_ENABLED) &&
+ !(port->rs485.flags & SER_RS485_RX_DURING_TX)) ||
+ port->iso7816.flags & SER_ISO7816_ENABLED)
atmel_stop_rx(port);
if (atmel_use_pdc_tx(port))
@@ -799,8 +955,9 @@ static void atmel_complete_tx_dma(void *arg)
*/
if (!uart_circ_empty(xmit))
atmel_tasklet_schedule(atmel_port, &atmel_port->tasklet_tx);
- else if ((port->rs485.flags & SER_RS485_ENABLED) &&
- !(port->rs485.flags & SER_RS485_RX_DURING_TX)) {
+ else if (((port->rs485.flags & SER_RS485_ENABLED) &&
+ !(port->rs485.flags & SER_RS485_RX_DURING_TX)) ||
+ port->iso7816.flags & SER_ISO7816_ENABLED) {
/* DMA done, stop TX, start RX for RS485 */
atmel_start_rx(port);
}
@@ -1281,6 +1438,9 @@ atmel_handle_status(struct uart_port *port, unsigned int pending,
wake_up_interruptible(&port->state->port.delta_msr_wait);
}
}
+
+ if (pending & (ATMEL_US_NACK | ATMEL_US_ITERATION))
+ dev_dbg(port->dev, "ISO7816 ERROR (0x%08x)\n", pending);
}
/*
@@ -1373,8 +1533,9 @@ static void atmel_tx_pdc(struct uart_port *port)
atmel_uart_writel(port, ATMEL_US_IER,
atmel_port->tx_done_mask);
} else {
- if ((port->rs485.flags & SER_RS485_ENABLED) &&
- !(port->rs485.flags & SER_RS485_RX_DURING_TX)) {
+ if (((port->rs485.flags & SER_RS485_ENABLED) &&
+ !(port->rs485.flags & SER_RS485_RX_DURING_TX)) ||
+ port->iso7816.flags & SER_ISO7816_ENABLED) {
/* DMA done, stop TX, start RX for RS485 */
atmel_start_rx(port);
}
@@ -2099,6 +2260,17 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
atmel_uart_writel(port, ATMEL_US_TTGR,
port->rs485.delay_rts_after_send);
mode |= ATMEL_US_USMODE_RS485;
+ } else if (port->iso7816.flags & SER_ISO7816_ENABLED) {
+ atmel_uart_writel(port, ATMEL_US_TTGR, port->iso7816.tg);
+ /* select mck clock, and output */
+ mode |= ATMEL_US_USCLKS_MCK | ATMEL_US_CLKO;
+ /* set max iterations */
+ mode |= ATMEL_US_MAX_ITER(3);
+ if ((port->iso7816.flags & SER_ISO7816_T_PARAM)
+ == SER_ISO7816_T(0))
+ mode |= ATMEL_US_USMODE_ISO7816_T0;
+ else
+ mode |= ATMEL_US_USMODE_ISO7816_T1;
} else if (termios->c_cflag & CRTSCTS) {
/* RS232 with hardware handshake (RTS/CTS) */
if (atmel_use_fifo(port) &&
@@ -2175,7 +2347,8 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
}
quot = cd | fp << ATMEL_US_FP_OFFSET;
- atmel_uart_writel(port, ATMEL_US_BRGR, quot);
+ if (!(port->iso7816.flags & SER_ISO7816_ENABLED))
+ atmel_uart_writel(port, ATMEL_US_BRGR, quot);
atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_RSTSTA | ATMEL_US_RSTRX);
atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN | ATMEL_US_RXEN);
atmel_port->tx_stopped = false;
@@ -2355,6 +2528,7 @@ static int atmel_init_port(struct atmel_uart_port *atmel_port,
port->mapbase = pdev->resource[0].start;
port->irq = pdev->resource[1].start;
port->rs485_config = atmel_config_rs485;
+ port->iso7816_config = atmel_config_iso7816;
port->membase = NULL;
memset(&atmel_port->rx_ring, 0, sizeof(atmel_port->rx_ring));
@@ -2378,8 +2552,12 @@ static int atmel_init_port(struct atmel_uart_port *atmel_port,
/* only enable clock when USART is in use */
}
- /* Use TXEMPTY for interrupt when rs485 else TXRDY or ENDTX|TXBUFE */
- if (port->rs485.flags & SER_RS485_ENABLED)
+ /*
+ * Use TXEMPTY for interrupt when rs485 or ISO7816 else TXRDY or
+ * ENDTX|TXBUFE
+ */
+ if (port->rs485.flags & SER_RS485_ENABLED ||
+ port->iso7816.flags & SER_ISO7816_ENABLED)
atmel_port->tx_done_mask = ATMEL_US_TXEMPTY;
else if (atmel_use_pdc_tx(port)) {
port->fifosize = PDC_BUFFER_SIZE;
@@ -2719,6 +2897,11 @@ static int atmel_serial_probe(struct platform_device *pdev)
}
atmel_port = &atmel_ports[ret];
+ atmel_port->pdata = of_device_get_match_data(&pdev->dev);
+ if (!atmel_port->pdata) {
+ ret = -EINVAL;
+ goto err;
+ }
atmel_port->backup_imr = 0;
atmel_port->uart.line = ret;
atmel_serial_probe_fifos(atmel_port, pdev);
diff --git a/drivers/tty/serial/atmel_serial.h b/drivers/tty/serial/atmel_serial.h
index ba3a2437cde4..6911177964ad 100644
--- a/drivers/tty/serial/atmel_serial.h
+++ b/drivers/tty/serial/atmel_serial.h
@@ -78,7 +78,8 @@
#define ATMEL_US_OVER BIT(19) /* Oversampling Mode */
#define ATMEL_US_INACK BIT(20) /* Inhibit Non Acknowledge */
#define ATMEL_US_DSNACK BIT(21) /* Disable Successive NACK */
-#define ATMEL_US_MAX_ITER GENMASK(26, 24) /* Max Iterations */
+#define ATMEL_US_MAX_ITER_MASK GENMASK(26, 24) /* Max Iterations */
+#define ATMEL_US_MAX_ITER(n) (((n) << 24) & ATMEL_US_MAX_ITER_MASK)
#define ATMEL_US_FILTER BIT(28) /* Infrared Receive Line Filter */
#define ATMEL_US_IER 0x08 /* Interrupt Enable Register */
@@ -124,7 +125,8 @@
#define ATMEL_US_TTGR 0x28 /* Transmitter Timeguard Register */
#define ATMEL_US_TG GENMASK(7, 0) /* Timeguard Value */
-#define ATMEL_US_FIDI 0x40 /* FI DI Ratio Register */
+#define ATMEL_US_FIDIR 0x40 /* FI DI Ratio Register */
+#define ATMEL_US_FIDI GENMASK(15, 0) /* FIDI ratio */
#define ATMEL_US_NER 0x44 /* Number of Errors Register */
#define ATMEL_US_IF 0x4c /* IrDA Filter Register */
--
2.12.2
^ permalink raw reply related
* [PATCH v3 1/2] tty/serial_core: add ISO7816 infrastructure
From: Ludovic Desroches @ 2018-08-07 13:00 UTC (permalink / raw)
To: linux-serial, linux-arch, linux-arm-kernel
Cc: gregkh, jslaby, arnd, richard.genoud, nicolas.ferre,
alexandre.belloni, linux-kernel, Ludovic Desroches
In-Reply-To: <20180807130049.5957-1-ludovic.desroches@microchip.com>
From: Nicolas Ferre <nicolas.ferre@microchip.com>
Add the ISO7816 ioctl and associated accessors and data structure.
Drivers can then use this common implementation to handle ISO7816.
Signed-off-by: Nicolas Ferre <nicolas.ferre@microchip.com>
[ludovic.desroches@microchip.com: squash and rebase, removal of gpios, checkpatch fixes]
Signed-off-by: Ludovic Desroches <ludovic.desroches@microchip.com>
---
arch/alpha/include/uapi/asm/ioctls.h | 2 ++
arch/mips/include/uapi/asm/ioctls.h | 2 ++
arch/parisc/include/uapi/asm/ioctls.h | 2 ++
arch/powerpc/include/uapi/asm/ioctls.h | 2 ++
arch/sh/include/uapi/asm/ioctls.h | 2 ++
arch/sparc/include/uapi/asm/ioctls.h | 2 ++
arch/xtensa/include/uapi/asm/ioctls.h | 2 ++
drivers/tty/serial/serial_core.c | 60 ++++++++++++++++++++++++++++++++++
include/linux/serial_core.h | 3 ++
include/uapi/asm-generic/ioctls.h | 2 ++
include/uapi/linux/serial.h | 17 ++++++++++
11 files changed, 96 insertions(+)
diff --git a/arch/alpha/include/uapi/asm/ioctls.h b/arch/alpha/include/uapi/asm/ioctls.h
index 3729d92d3fa8..1e9121c9b3c7 100644
--- a/arch/alpha/include/uapi/asm/ioctls.h
+++ b/arch/alpha/include/uapi/asm/ioctls.h
@@ -102,6 +102,8 @@
#define TIOCGPTLCK _IOR('T', 0x39, int) /* Get Pty lock state */
#define TIOCGEXCL _IOR('T', 0x40, int) /* Get exclusive mode state */
#define TIOCGPTPEER _IO('T', 0x41) /* Safely open the slave */
+#define TIOCGISO7816 _IOR('T', 0x42, struct serial_iso7816)
+#define TIOCSISO7816 _IOWR('T', 0x43, struct serial_iso7816)
#define TIOCSERCONFIG 0x5453
#define TIOCSERGWILD 0x5454
diff --git a/arch/mips/include/uapi/asm/ioctls.h b/arch/mips/include/uapi/asm/ioctls.h
index 890245a9f0c4..16aa8a766aec 100644
--- a/arch/mips/include/uapi/asm/ioctls.h
+++ b/arch/mips/include/uapi/asm/ioctls.h
@@ -93,6 +93,8 @@
#define TIOCGPTLCK _IOR('T', 0x39, int) /* Get Pty lock state */
#define TIOCGEXCL _IOR('T', 0x40, int) /* Get exclusive mode state */
#define TIOCGPTPEER _IO('T', 0x41) /* Safely open the slave */
+#define TIOCGISO7816 _IOR('T', 0x42, struct serial_iso7816)
+#define TIOCSISO7816 _IOWR('T', 0x43, struct serial_iso7816)
/* I hope the range from 0x5480 on is free ... */
#define TIOCSCTTY 0x5480 /* become controlling tty */
diff --git a/arch/parisc/include/uapi/asm/ioctls.h b/arch/parisc/include/uapi/asm/ioctls.h
index aafb1c0ca0af..82d1148c6379 100644
--- a/arch/parisc/include/uapi/asm/ioctls.h
+++ b/arch/parisc/include/uapi/asm/ioctls.h
@@ -62,6 +62,8 @@
#define TIOCGPTLCK _IOR('T', 0x39, int) /* Get Pty lock state */
#define TIOCGEXCL _IOR('T', 0x40, int) /* Get exclusive mode state */
#define TIOCGPTPEER _IO('T', 0x41) /* Safely open the slave */
+#define TIOCGISO7816 _IOR('T', 0x42, struct serial_iso7816)
+#define TIOCSISO7816 _IOWR('T', 0x43, struct serial_iso7816)
#define FIONCLEX 0x5450 /* these numbers need to be adjusted. */
#define FIOCLEX 0x5451
diff --git a/arch/powerpc/include/uapi/asm/ioctls.h b/arch/powerpc/include/uapi/asm/ioctls.h
index 41b1a5c15734..2c145da3b774 100644
--- a/arch/powerpc/include/uapi/asm/ioctls.h
+++ b/arch/powerpc/include/uapi/asm/ioctls.h
@@ -102,6 +102,8 @@
#define TIOCGPTLCK _IOR('T', 0x39, int) /* Get Pty lock state */
#define TIOCGEXCL _IOR('T', 0x40, int) /* Get exclusive mode state */
#define TIOCGPTPEER _IO('T', 0x41) /* Safely open the slave */
+#define TIOCGISO7816 _IOR('T', 0x42, struct serial_iso7816)
+#define TIOCSISO7816 _IOWR('T', 0x43, struct serial_iso7816)
#define TIOCSERCONFIG 0x5453
#define TIOCSERGWILD 0x5454
diff --git a/arch/sh/include/uapi/asm/ioctls.h b/arch/sh/include/uapi/asm/ioctls.h
index cc62f6f98103..11866d4f60e1 100644
--- a/arch/sh/include/uapi/asm/ioctls.h
+++ b/arch/sh/include/uapi/asm/ioctls.h
@@ -95,6 +95,8 @@
#define TIOCGPTLCK _IOR('T', 0x39, int) /* Get Pty lock state */
#define TIOCGEXCL _IOR('T', 0x40, int) /* Get exclusive mode state */
#define TIOCGPTPEER _IO('T', 0x41) /* Safely open the slave */
+#define TIOCGISO7816 _IOR('T', 0x42, struct serial_iso7816)
+#define TIOCSISO7816 _IOWR('T', 0x43, struct serial_iso7816)
#define TIOCSERCONFIG _IO('T', 83) /* 0x5453 */
#define TIOCSERGWILD _IOR('T', 84, int) /* 0x5454 */
diff --git a/arch/sparc/include/uapi/asm/ioctls.h b/arch/sparc/include/uapi/asm/ioctls.h
index 2df52711e170..7fd2f5873c9e 100644
--- a/arch/sparc/include/uapi/asm/ioctls.h
+++ b/arch/sparc/include/uapi/asm/ioctls.h
@@ -27,6 +27,8 @@
#define TIOCGEXCL _IOR('T', 0x40, int) /* Get exclusive mode state */
#define TIOCGRS485 _IOR('T', 0x41, struct serial_rs485)
#define TIOCSRS485 _IOWR('T', 0x42, struct serial_rs485)
+#define TIOCGISO7816 _IOR('T', 0x43, struct serial_iso7816)
+#define TIOCSISO7816 _IOWR('T', 0x44, struct serial_iso7816)
/* Note that all the ioctls that are not available in Linux have a
* double underscore on the front to: a) avoid some programs to
diff --git a/arch/xtensa/include/uapi/asm/ioctls.h b/arch/xtensa/include/uapi/asm/ioctls.h
index ec43609cbfc5..6d4a87296c95 100644
--- a/arch/xtensa/include/uapi/asm/ioctls.h
+++ b/arch/xtensa/include/uapi/asm/ioctls.h
@@ -107,6 +107,8 @@
#define TIOCGPTLCK _IOR('T', 0x39, int) /* Get Pty lock state */
#define TIOCGEXCL _IOR('T', 0x40, int) /* Get exclusive mode state */
#define TIOCGPTPEER _IO('T', 0x41) /* Safely open the slave */
+#define TIOCGISO7816 _IOR('T', 0x42, struct serial_iso7816)
+#define TIOCSISO7816 _IOWR('T', 0x43, struct serial_iso7816)
#define TIOCSERCONFIG _IO('T', 83)
#define TIOCSERGWILD _IOR('T', 84, int)
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 9c14a453f73c..af8d5b12fba9 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1301,6 +1301,58 @@ static int uart_set_rs485_config(struct uart_port *port,
return 0;
}
+static int uart_get_iso7816_config(struct uart_port *port,
+ struct serial_iso7816 __user *iso7816)
+{
+ unsigned long flags;
+ struct serial_iso7816 aux;
+
+ if (!port->iso7816_config)
+ return -ENOIOCTLCMD;
+
+ spin_lock_irqsave(&port->lock, flags);
+ aux = port->iso7816;
+ spin_unlock_irqrestore(&port->lock, flags);
+
+ if (copy_to_user(iso7816, &aux, sizeof(aux)))
+ return -EFAULT;
+
+ return 0;
+}
+
+static int uart_set_iso7816_config(struct uart_port *port,
+ struct serial_iso7816 __user *iso7816_user)
+{
+ struct serial_iso7816 iso7816;
+ int i, ret;
+ unsigned long flags;
+
+ if (!port->iso7816_config)
+ return -ENOIOCTLCMD;
+
+ if (copy_from_user(&iso7816, iso7816_user, sizeof(*iso7816_user)))
+ return -EFAULT;
+
+ /*
+ * There are 5 words reserved for future use. Check that userspace
+ * doesn't put stuff in there to prevent breakages in the future.
+ */
+ for (i = 0; i < 5; i++)
+ if (iso7816.reserved[i])
+ return -EINVAL;
+
+ spin_lock_irqsave(&port->lock, flags);
+ ret = port->iso7816_config(port, &iso7816);
+ spin_unlock_irqrestore(&port->lock, flags);
+ if (ret)
+ return ret;
+
+ if (copy_to_user(iso7816_user, &port->iso7816, sizeof(port->iso7816)))
+ return -EFAULT;
+
+ return 0;
+}
+
/*
* Called via sys_ioctl. We can use spin_lock_irq() here.
*/
@@ -1385,6 +1437,14 @@ uart_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg)
case TIOCSRS485:
ret = uart_set_rs485_config(uport, uarg);
break;
+
+ case TIOCSISO7816:
+ ret = uart_set_iso7816_config(state->uart_port, uarg);
+ break;
+
+ case TIOCGISO7816:
+ ret = uart_get_iso7816_config(state->uart_port, uarg);
+ break;
default:
if (uport->ops->ioctl)
ret = uport->ops->ioctl(uport, cmd, arg);
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 06ea4eeb09ab..d6e7747ffd46 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -137,6 +137,8 @@ struct uart_port {
void (*handle_break)(struct uart_port *);
int (*rs485_config)(struct uart_port *,
struct serial_rs485 *rs485);
+ int (*iso7816_config)(struct uart_port *,
+ struct serial_iso7816 *iso7816);
unsigned int irq; /* irq number */
unsigned long irqflags; /* irq flags */
unsigned int uartclk; /* base uart clock */
@@ -253,6 +255,7 @@ struct uart_port {
struct attribute_group *attr_group; /* port specific attributes */
const struct attribute_group **tty_groups; /* all attributes (serial core use only) */
struct serial_rs485 rs485;
+ struct serial_iso7816 iso7816;
void *private_data; /* generic platform data pointer */
};
diff --git a/include/uapi/asm-generic/ioctls.h b/include/uapi/asm-generic/ioctls.h
index 040651735662..cdc9f4ca8c27 100644
--- a/include/uapi/asm-generic/ioctls.h
+++ b/include/uapi/asm-generic/ioctls.h
@@ -79,6 +79,8 @@
#define TIOCGPTLCK _IOR('T', 0x39, int) /* Get Pty lock state */
#define TIOCGEXCL _IOR('T', 0x40, int) /* Get exclusive mode state */
#define TIOCGPTPEER _IO('T', 0x41) /* Safely open the slave */
+#define TIOCGISO7816 _IOR('T', 0x42, struct serial_iso7816)
+#define TIOCSISO7816 _IOWR('T', 0x43, struct serial_iso7816)
#define FIONCLEX 0x5450
#define FIOCLEX 0x5451
diff --git a/include/uapi/linux/serial.h b/include/uapi/linux/serial.h
index 3fdd0dee8b41..93eb3c496ff1 100644
--- a/include/uapi/linux/serial.h
+++ b/include/uapi/linux/serial.h
@@ -132,4 +132,21 @@ struct serial_rs485 {
are a royal PITA .. */
};
+/*
+ * Serial interface for controlling ISO7816 settings on chips with suitable
+ * support. Set with TIOCSISO7816 and get with TIOCGISO7816 if supported by
+ * your platform.
+ */
+struct serial_iso7816 {
+ __u32 flags; /* ISO7816 feature flags */
+#define SER_ISO7816_ENABLED (1 << 0)
+#define SER_ISO7816_T_PARAM (0x0f << 4)
+#define SER_ISO7816_T(t) (((t) & 0x0f) << 4)
+ __u32 tg;
+ __u32 sc_fi;
+ __u32 sc_di;
+ __u32 clk;
+ __u32 reserved[5];
+};
+
#endif /* _UAPI_LINUX_SERIAL_H */
--
2.12.2
^ permalink raw reply related
* [PATCH v3 0/2] add ISO7816 support
From: Ludovic Desroches @ 2018-08-07 13:00 UTC (permalink / raw)
To: linux-serial, linux-arch, linux-arm-kernel
Cc: gregkh, jslaby, arnd, richard.genoud, nicolas.ferre,
alexandre.belloni, linux-kernel, Ludovic Desroches
Hi,
This patchset adds support for the ISO7816 standard. The USART devices in
Microchip SoCs have an ISO7816 mode. It allows to let the USART managing
the CLK and I/O signals of a smart card.
Changes:
- v3
- add IOCTLs to parisc arch.
- rework atmel_config_iso7816 according to the comments by Richard
- keep max iteration hard coded. Will see later if the user need to set
this value. It may also concern the normal/inverse mode.
- improve the checking of the value from the user space.
- v2
- uart_get_iso7816_config: check there is an iso7816_config function
- use IOCTL macros to generate the IOCTL number
- check that reserved field is not used
- remove debug logs
- check that the iso7816_config is right before doing any action
- change the error from nack and max iteration status to a debug message
- remove patch 3 as it concerns both rs485 and iso7816 to think more
about the need of adding a lock or not
Nicolas Ferre (2):
tty/serial_core: add ISO7816 infrastructure
tty/serial: atmel: add ISO7816 support
arch/alpha/include/uapi/asm/ioctls.h | 2 +
arch/mips/include/uapi/asm/ioctls.h | 2 +
arch/parisc/include/uapi/asm/ioctls.h | 2 +
arch/powerpc/include/uapi/asm/ioctls.h | 2 +
arch/sh/include/uapi/asm/ioctls.h | 2 +
arch/sparc/include/uapi/asm/ioctls.h | 2 +
arch/xtensa/include/uapi/asm/ioctls.h | 2 +
drivers/tty/serial/atmel_serial.c | 211 ++++++++++++++++++++++++++++++---
drivers/tty/serial/atmel_serial.h | 6 +-
drivers/tty/serial/serial_core.c | 60 ++++++++++
include/linux/serial_core.h | 3 +
include/uapi/asm-generic/ioctls.h | 2 +
include/uapi/linux/serial.h | 17 +++
13 files changed, 297 insertions(+), 16 deletions(-)
--
2.12.2
^ permalink raw reply
* [PATCH v2] serial: mxs-auart: Fix potential infinite loop
From: Anton Vasilyev @ 2018-08-07 10:59 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Anton Vasilyev, Jiri Slaby, linux-serial, linux-kernel,
ldv-project
In-Reply-To: <20180806164346.9986-1-vasilyev@ispras.ru>
On the error path of mxs_auart_request_gpio_irq() is performed
backward iterating with index i of enum type. Underline enum type
may be unsigned char. In this case check (--i >= 0) will be always
true and error handling goes into infinite loop.
The patch changes the check so that it is valid for signed and unsigned
types.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Anton Vasilyev <vasilyev@ispras.ru>
---
v2: mistype in patch as result of combination of different fixes.
Change comment and leave enum type.
---
drivers/tty/serial/mxs-auart.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
index 76aa289652f7..27235a526cce 100644
--- a/drivers/tty/serial/mxs-auart.c
+++ b/drivers/tty/serial/mxs-auart.c
@@ -1634,8 +1634,9 @@ static int mxs_auart_request_gpio_irq(struct mxs_auart_port *s)
/*
* If something went wrong, rollback.
+ * Be careful: i may be unsigned.
*/
- while (err && (--i >= 0))
+ while (err && (i-- > 0))
if (irq[i] >= 0)
free_irq(irq[i], s);
--
2.18.0
^ permalink raw reply related
* Re: [PATCH v2 14/18] serial: intel: Add CCF support
From: Geert Uytterhoeven @ 2018-08-07 7:33 UTC (permalink / raw)
To: songjun.wu
Cc: Arnd Bergmann, Greg KH, Hauke Mehrtens, hua.ma, yixin.zhu,
chuanhua.lei, qi-ming.wu, Linux MIPS Mailing List, linux-clk,
open list:SERIAL DRIVERS,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux Kernel Mailing List, Jiri Slaby
In-Reply-To: <d6d0be20-aea9-cecf-1e39-3d65c0dbad5f@linux.intel.com>
Hi Songjun,
On Tue, Aug 7, 2018 at 9:18 AM Wu, Songjun <songjun.wu@linux.intel.com> wrote:
> On 8/6/2018 5:29 PM, Geert Uytterhoeven wrote:
> > On Mon, Aug 6, 2018 at 10:58 AM Wu, Songjun <songjun.wu@linux.intel.com> wrote:
> >> The reason we add a new head file is also for two macros(LTQ_EARLY_ASC
> >> and CPHYSADDR)
> >> used by legacy product. We need to provide the no-op stub for these two
> >> macro for new product.
> > No you don't. The line number should not be obtained by comparing the
> > resource address with a hardcoded base address.
> This is the previous code. Now the line number is obtained from dts.
Note that obtaining line numbers from DTS has its own share of problems, when
considering DT overlays. I've replied to the patch adding the call to
of_alias_get_id().
> We keep this code for the compatibility.
>
> Referring to the conditional-compilation part in coding-style,
> We add a header file to avoid using “#ifdef” in C file.
> > Perhaps the override of port->line should just be removed, as IIRC, the serial
> > core has already filled in that field with the (next available) line number?
Gr{oetje,eeting}s,
Geert--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v2 08/18] serial: intel: Get serial id from dts
From: Geert Uytterhoeven @ 2018-08-07 7:33 UTC (permalink / raw)
To: songjun.wu
Cc: hua.ma, yixin.zhu, chuanhua.lei, qi-ming.wu,
Linux MIPS Mailing List, linux-clk, open list:SERIAL DRIVERS,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Greg KH, Linux Kernel Mailing List, Jiri Slaby
In-Reply-To: <20180803030237.3366-9-songjun.wu@linux.intel.com>
Hi Songjun,
On Fri, Aug 3, 2018 at 5:04 AM Songjun Wu <songjun.wu@linux.intel.com> wrote:
> Get serial id from dts.
>
> "#ifdef CONFIG_LANTIQ" preprocessor is used because LTQ_EARLY_ASC
> macro is defined in lantiq_soc.h.
> lantiq_soc.h is in arch path for legacy product support.
>
> arch/mips/include/asm/mach-lantiq/xway/lantiq_soc.h
>
> If "#ifdef preprocessor" is changed to
> "if (IS_ENABLED(CONFIG_LANTIQ))", when CONFIG_LANTIQ is not enabled,
> code using LTQ_EARLY_ASC is compiled.
> Compilation will fail for no LTQ_EARLY_ASC defined.
>
> Signed-off-by: Songjun Wu <songjun.wu@linux.intel.com>
Thanks for your patch!
> @@ -699,9 +700,19 @@ lqasc_probe(struct platform_device *pdev)
> return -ENODEV;
> }
>
> - /* check if this is the console port */
> - if (mmres->start != CPHYSADDR(LTQ_EARLY_ASC))
> - line = 1;
> + /* get serial id */
> + line = of_alias_get_id(node, "serial");
> + if (line < 0) {
> +#ifdef CONFIG_LANTIQ
> + if (mmres->start == CPHYSADDR(LTQ_EARLY_ASC))
> + line = 0;
> + else
> + line = 1;
> +#else
> + dev_err(&pdev->dev, "failed to get alias id, errno %d\n", line);
> + return line;
Please note that not providing a fallback here makes life harder when using
DT overlays.
See the description of commit 7678f4c20fa7670f ("serial: sh-sci: Add support
for dynamic instances") for background info.
> +#endif
> + }
>
> if (lqasc_port[line]) {
> dev_err(&pdev->dev, "port %d already allocated\n", line);
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v2 14/18] serial: intel: Add CCF support
From: Wu, Songjun @ 2018-08-07 7:18 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Arnd Bergmann, Greg KH, Hauke Mehrtens, hua.ma, yixin.zhu,
chuanhua.lei, qi-ming.wu, Linux MIPS Mailing List, linux-clk,
open list:SERIAL DRIVERS,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux Kernel Mailing List, Jiri Slaby
In-Reply-To: <CAMuHMdUJ8-bveoHWOetjrHtq2HNPnf00PidQwJ-kZD2o86KLMw@mail.gmail.com>
On 8/6/2018 5:29 PM, Geert Uytterhoeven wrote:
> Hi Songjun,
>
> On Mon, Aug 6, 2018 at 10:58 AM Wu, Songjun <songjun.wu@linux.intel.com> wrote:
>> On 8/6/2018 3:20 PM, Geert Uytterhoeven wrote:
>>> On Mon, Aug 6, 2018 at 9:15 AM Wu, Songjun <songjun.wu@linux.intel.com> wrote:
>>>> On 8/5/2018 5:03 AM, Arnd Bergmann wrote:
>>>>> On Sat, Aug 4, 2018 at 2:43 PM, Greg Kroah-Hartman
>>>>> <gregkh@linuxfoundation.org> wrote:
>>>>>> On Sat, Aug 04, 2018 at 12:54:22PM +0200, Hauke Mehrtens wrote:
>>>>>>> On 08/03/2018 12:30 PM, Greg Kroah-Hartman wrote:
>>>>>>>> On Fri, Aug 03, 2018 at 03:33:38PM +0800, Wu, Songjun wrote:
>>>>>>> This patch makes it possible to use it with the legacy lantiq code and
>>>>>>> also with the common clock framework. I see multiple options to fix this
>>>>>>> problem.
>>>>>>>
>>>>>>> 1. The current approach to have it as a compile variant for a) legacy
>>>>>>> lantiq arch code without common clock framework and b) support for SoCs
>>>>>>> using the common clock framework.
>>>>>>> 2. Convert the lantiq arch code to the common clock framework. This
>>>>>>> would be a good approach, but it need some efforts.
>>>>>>> 3. Remove the arch/mips/lantiq code. There are still users of this code.
>>>>>>> 4. Use the old APIs also for the new xRX500 SoC, I do not like this
>>>>>>> approach.
>>>>>>> 5. Move lantiq_soc.h to somewhere in include/linux/ so it is globally
>>>>>>> available and provide some better wrapper code.
>>>>>> I don't really care what you do at this point in time, but you all
>>>>>> should know better than the crazy #ifdef is not allowed to try to
>>>>>> prevent/allow the inclusion of a .h file. Checkpatch might have even
>>>>>> warned you about it, right?
>>>>>>
>>>>>> So do it correctly, odds are #5 is correct, as that makes it work like
>>>>>> any other device in the kernel. You are not unique here.
>>>>> The best approach here would clearly be 2. We don't want platform
>>>>> specific header files for doing things that should be completely generic.
>>>>>
>>>>> Converting lantiq to the common-clk framework obviously requires
>>>>> some work, but then again the whole arch/mips/lantiq/clk.c file
>>>>> is fairly short and maybe not that hard to convert.
>>>>>
>>>>> >From looking at arch/mips/lantiq/xway/sysctrl.c, it appears that you
>>>>> already use the clkdev lookup mechanism for some devices without
>>>>> using COMMON_CLK, so I would assume that you can also use those
>>>>> for the remaining clks, which would be much simpler. It registers
>>>>> one anonymous clk there as
>>>>>
>>>>> clkdev_add_pmu("1e100c00.serial", NULL, 0, 0, PMU_ASC1);
>>>>>
>>>>> so why not add replace that with two named clocks and just use
>>>>> the same names in the DT for the newer chip?
>>>>>
>>>>> Arnd
>>>> We discussed internally and have another solution for this issue.
>>>> Add one lantiq.h in the serial folder, and use "#ifdef preprocessor" in
>>>> lantiq.h,
>>>> also providing no-op stub functions in the #else case, then call those
>>>> functions
>>>> unconditionally from lantiq.c to avoid #ifdef in C file.
>>>>
>>>> To support CCF in legacy product is another topic, is not included in
>>>> this patch.
>>>>
>>>> The implementation is as following：
>>>> #ifdef CONFIG_LANTIQ
>>>> #include <lantiq_soc.h>
>>>> #else
>>>> #define LTQ_EARLY_ASC 0
>>>> #define CPHYSADDR(_val) 0
>>>>
>>>> static inline struct clk *clk_get_fpi(void)
>>>> {
>>>> return NULL;
>>>> }
>>>> #endif
>>> Why not use clkdev_add(), as Arnd suggested?
>>> That would be a 3-line patch without introducing a new header file and an ugly
>>> #ifdef, which complicates compile coverage testing?
>>>
>> The reason we add a new head file is also for two macros(LTQ_EARLY_ASC
>> and CPHYSADDR)
>> used by legacy product. We need to provide the no-op stub for these two
>> macro for new product.
> No you don't. The line number should not be obtained by comparing the
> resource address with a hardcoded base address.
This is the previous code. Now the line number is obtained from dts.
We keep this code for the compatibility.
Referring to the conditional-compilation part in coding-style,
We add a header file to avoid using “#ifdef” in C file.
> Perhaps the override of port->line should just be removed, as IIRC, the serial
> core has already filled in that field with the (next available) line number?
>
> Gr{oetje,eeting}s,
>
> Geert
^ permalink raw reply
* [PATCH] serial: mxs-auart: Fix potential infinite loop
From: Anton Vasilyev @ 2018-08-06 16:43 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Anton Vasilyev, Jiri Slaby, linux-serial, linux-kernel,
ldv-project
On the error path of mxs_auart_request_gpio_irq() is performed
backward iterating with index i of enum type. Underline enum type
may be unsigned char. In this case check (--i >= 0) will be always
true and error handling goes into infinite loop.
The patch changes type of index variable to int.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Anton Vasilyev <vasilyev@ispras.ru>
---
drivers/tty/serial/mxs-auart.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
index 76aa289652f7..89b34bb09cde 100644
--- a/drivers/tty/serial/mxs-auart.c
+++ b/drivers/tty/serial/mxs-auart.c
@@ -1617,7 +1617,7 @@ static void mxs_auart_free_gpio_irq(struct mxs_auart_port *s)
static int mxs_auart_request_gpio_irq(struct mxs_auart_port *s)
{
int *irq = s->gpio_irq;
- enum mctrl_gpio_idx i;
+ int i;
int err = 0;
for (i = 0; (i < UART_GPIO_MAX) && !err; i++) {
@@ -1634,8 +1634,9 @@ static int mxs_auart_request_gpio_irq(struct mxs_auart_port *s)
/*
* If something went wrong, rollback.
+ * Be careful: i may be unsigned.
*/
- while (err && (--i >= 0))
+ while (err && (i-- > 0))
if (irq[i] >= 0)
free_irq(irq[i], s);
--
2.18.0
^ permalink raw reply related
* Re: [RFC] serial: sc16is7xx: Use DT sub-nodes for UART ports
From: Andreas Färber @ 2018-08-06 16:21 UTC (permalink / raw)
To: linux-serial, Rob Herring
Cc: linux-mips, jringle, allsey87, Jakub Kicinski, Xue Liu,
Greg Kroah-Hartman, Jiri Slaby, linux-kernel
In-Reply-To: <20180805232651.10605-1-afaerber@suse.de>
Am 06.08.2018 um 01:26 schrieb Andreas Färber:
> This is to allow using serdev.
>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
> drivers/tty/serial/sc16is7xx.c | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
> index 243c96025053..ad7267274f65 100644
> --- a/drivers/tty/serial/sc16is7xx.c
> +++ b/drivers/tty/serial/sc16is7xx.c
> @@ -1213,9 +1213,31 @@ static int sc16is7xx_probe(struct device *dev,
> SC16IS7XX_IOCONTROL_SRESET_BIT);
>
> for (i = 0; i < devtype->nr_uart; ++i) {
> +#ifdef CONFIG_OF
Looks like this and below need to be CONFIG_OF_ADDRESS (build failure
reported for sparc).
Regards,
Andreas
> + struct device_node *np;
> + struct platform_device *pdev;
> + char name[6] = "uartx";
> +#endif
> +
> s->p[i].line = i;
> /* Initialize port data */
> +#ifdef CONFIG_OF
> + name[4] = '0' + i;
> + np = of_get_child_by_name(dev->of_node, name);
> + if (IS_ERR(np)) {
> + ret = PTR_ERR(np);
> + goto out_ports;
> + }
> + pdev = of_platform_device_create(np, NULL, dev);
> + if (IS_ERR(pdev)) {
> + ret = PTR_ERR(pdev);
> + goto out_ports;
> + }
> + platform_set_drvdata(pdev, dev_get_drvdata(dev));
> + s->p[i].port.dev = &pdev->dev;
> +#else
> s->p[i].port.dev = dev;
> +#endif
> s->p[i].port.irq = irq;
> s->p[i].port.type = PORT_SC16IS7XX;
> s->p[i].port.fifosize = SC16IS7XX_FIFO_SIZE;
> @@ -1271,6 +1293,9 @@ static int sc16is7xx_probe(struct device *dev,
> for (i--; i >= 0; i--) {
> uart_remove_one_port(&sc16is7xx_uart, &s->p[i].port);
> clear_bit(s->p[i].port.line, &sc16is7xx_lines);
> +#ifdef CONFIG_OF
> + of_platform_device_destroy(s->p[i].port.dev, NULL);
> +#endif
> }
>
> #ifdef CONFIG_GPIOLIB
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
^ permalink raw reply
* Re: [PATCH v2 02/18] clk: intel: Add clock driver for Intel MIPS SoCs
From: Rob Herring @ 2018-08-06 15:19 UTC (permalink / raw)
To: Songjun Wu
Cc: hua.ma, yixin zhu, chuanhua.lei, qi-ming.wu, Linux-MIPS,
linux-clk, open list:SERIAL DRIVERS, devicetree,
Michael Turquette, Stephen Boyd, linux-kernel@vger.kernel.org,
Mark Rutland
In-Reply-To: <20180803030237.3366-3-songjun.wu@linux.intel.com>
On Thu, Aug 2, 2018 at 9:03 PM Songjun Wu <songjun.wu@linux.intel.com> wrote:
>
> From: Yixin Zhu <yixin.zhu@linux.intel.com>
>
> This driver provides PLL clock registration as well as various clock
> branches, e.g. MUX clock, gate clock, divider clock and so on.
>
> PLLs that provide clock to DDR, CPU and peripherals are shown below:
>
> +---------+
> |--->| LCPLL3 0|--PCIe clk-->
> XO | +---------+
> +-----------|
> | +---------+
> | | 3|--PAE clk-->
> |--->| PLL0B 2|--GSWIP clk-->
> | | 1|--DDR clk-->DDR PHY clk-->
> | | 0|--CPU1 clk--+ +-----+
> | +---------+ |--->0 |
> | | MUX |--CPU clk-->
> | +---------+ |--->1 |
> | | 0|--CPU0 clk--+ +-----+
> |--->| PLLOA 1|--SSX4 clk-->
> | 2|--NGI clk-->
> | 3|--CBM clk-->
> +---------+
>
> Signed-off-by: Yixin Zhu <yixin.zhu@linux.intel.com>
> Signed-off-by: Songjun Wu <songjun.wu@linux.intel.com>
> ---
>
> Changes in v2:
> - Rewrite clock driver, add platform clock description details in
> clock driver.
>
> drivers/clk/Kconfig | 1 +
> drivers/clk/Makefile | 3 +
> drivers/clk/intel/Kconfig | 20 ++
> drivers/clk/intel/Makefile | 7 +
> drivers/clk/intel/clk-cgu-pll.c | 166 ++++++++++
> drivers/clk/intel/clk-cgu-pll.h | 34 ++
> drivers/clk/intel/clk-cgu.c | 470 +++++++++++++++++++++++++++
> drivers/clk/intel/clk-cgu.h | 259 +++++++++++++++
> drivers/clk/intel/clk-grx500.c | 168 ++++++++++
> include/dt-bindings/clock/intel,grx500-clk.h | 69 ++++
This belongs with the clk binding patch.
Rob
^ permalink raw reply
* Re: [PATCH v2 03/18] dt-bindings: clk: Add documentation of grx500 clock controller
From: Rob Herring @ 2018-08-06 15:18 UTC (permalink / raw)
To: Songjun Wu
Cc: hua.ma, yixin zhu, chuanhua.lei, qi-ming.wu, Linux-MIPS,
linux-clk, open list:SERIAL DRIVERS, devicetree,
Michael Turquette, Stephen Boyd, linux-kernel@vger.kernel.org,
Mark Rutland
In-Reply-To: <20180803030237.3366-4-songjun.wu@linux.intel.com>
On Thu, Aug 2, 2018 at 9:03 PM Songjun Wu <songjun.wu@linux.intel.com> wrote:
>
> From: Yixin Zhu <yixin.zhu@linux.intel.com>
>
> This patch adds binding documentation for grx500 clock controller.
>
> Signed-off-by: YiXin Zhu <yixin.zhu@linux.intel.com>
> Signed-off-by: Songjun Wu <songjun.wu@linux.intel.com>
> ---
>
> Changes in v2:
> - Rewrite clock driver's dt-binding document according to Rob Herring's
> comments.
> - Simplify device tree docoment, remove some clock description.
>
> .../devicetree/bindings/clock/intel,grx500-clk.txt | 39 ++++++++++++++++++++++
Please match the compatible string: intel,grx500-cgu.txt
> 1 file changed, 39 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/intel,grx500-clk.txt
>
> diff --git a/Documentation/devicetree/bindings/clock/intel,grx500-clk.txt b/Documentation/devicetree/bindings/clock/intel,grx500-clk.txt
> new file mode 100644
> index 000000000000..e54e1dad9196
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/intel,grx500-clk.txt
> @@ -0,0 +1,39 @@
> +Device Tree Clock bindings for grx500 PLL controller.
> +
> +This binding uses the common clock binding:
> + Documentation/devicetree/bindings/clock/clock-bindings.txt
> +
> +The grx500 clock controller supplies clock to various controllers within the
> +SoC.
> +
> +Required properties for clock node
> +- compatible: Should be "intel,grx500-cgu".
> +- reg: physical base address of the controller and length of memory range.
> +- #clock-cells: should be 1.
> +
> +Optional Propteries:
> +- intel,osc-frequency: frequency of the osc clock.
> +if missing, driver will use clock rate defined in the driver.
This should use a fixed-clock node instead.
> +
> +Example: Clock controller node:
> +
> + cgu: cgu@16200000 {
> + compatible = "intel,grx500-cgu", "syscon";
> + reg = <0x16200000 0x200>;
> + #clock-cells = <1>;
> + };
> +
> +
> +Example: UART controller node that consumes the clock generated by clock
> + controller.
> +
> + asc0: serial@16600000 {
> + compatible = "lantiq,asc";
> + reg = <0x16600000 0x100000>;
> + interrupt-parent = <&gic>;
> + interrupts = <GIC_SHARED 103 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SHARED 105 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SHARED 106 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&cgu CLK_SSX4>, <&cgu GCLK_UART>;
> + clock-names = "freq", "asc";
> + };
> --
> 2.11.0
>
^ permalink raw reply
* Re: [PATCH v2 05/18] dt-binding: MIPS: Add documentation of Intel MIPS SoCs
From: Rob Herring @ 2018-08-06 15:16 UTC (permalink / raw)
To: Songjun Wu
Cc: hua.ma, yixin zhu, chuanhua.lei, qi-ming.wu, Linux-MIPS,
linux-clk, open list:SERIAL DRIVERS, devicetree, James Hogan,
linux-kernel@vger.kernel.org, Paul Burton, Ralf Baechle,
Mark Rutland
In-Reply-To: <20180803030237.3366-6-songjun.wu@linux.intel.com>
On Thu, Aug 2, 2018 at 9:03 PM Songjun Wu <songjun.wu@linux.intel.com> wrote:
>
> From: Hua Ma <hua.ma@linux.intel.com>
>
> This patch adds binding documentation for the
> compatible values of the Intel MIPS SoCs.
>
> Signed-off-by: Hua Ma <hua.ma@linux.intel.com>
> Signed-off-by: Songjun Wu <songjun.wu@linux.intel.com>
> ---
>
> Changes in v2:
> - New patch split from previous patch
> - Add the board and chip compatible in dt document
>
> Documentation/devicetree/bindings/mips/intel.txt | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mips/intel.txt
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 06/18] MIPS: dts: Change upper case to lower case
From: Rob Herring @ 2018-08-06 15:14 UTC (permalink / raw)
To: Songjun Wu
Cc: hua.ma, yixin zhu, chuanhua.lei, qi-ming.wu, Linux-MIPS,
linux-clk, open list:SERIAL DRIVERS, devicetree, James Hogan,
linux-kernel@vger.kernel.org, Thomas Gleixner,
Philippe Ombredanne, Paul Burton, Kate Stewart,
Greg Kroah-Hartman, Mark Rutland, Ralf Baechle
In-Reply-To: <20180803030237.3366-7-songjun.wu@linux.intel.com>
On Thu, Aug 2, 2018 at 9:03 PM Songjun Wu <songjun.wu@linux.intel.com> wrote:
>
> All the upper case in unit-address and hex constants are
> changed to lower case according to the Linux conventions.
It is DT conventions, not Linux.
Otherwise,
Reviewed-by: Rob Herring <robh@kernel.org>
>
> Signed-off-by: Songjun Wu <songjun.wu@linux.intel.com>
> ---
>
> Changes in v2: None
>
> arch/mips/boot/dts/lantiq/danube.dtsi | 42 ++++++++++++++++-----------------
> arch/mips/boot/dts/lantiq/easy50712.dts | 14 +++++------
> 2 files changed, 28 insertions(+), 28 deletions(-)
^ permalink raw reply
* Re: [PATCH v5 7/7] dmaengine: imx-sdma: alloclate bd memory from dma pool
From: Lucas Stach @ 2018-08-06 12:44 UTC (permalink / raw)
To: Robin Gong, vkoul, s.hauer, dan.j.williams, gregkh, jslaby
Cc: linux-serial, dmaengine, linux-kernel, linux-arm-kernel,
linux-imx
In-Reply-To: <1529427424-12321-8-git-send-email-yibin.gong@nxp.com>
Hi Vinod, hi Robin,
this patch is already in your slave-dma tree, but upon closer
inspection this is totally doing the wrong thing and should be dropped,
see inline comments. The patchset in your tree will regress without
this patch, though. So I think we need to delay getting this set
upstream until this issue is properly fixed.
Am Mittwoch, den 20.06.2018, 00:57 +0800 schrieb Robin Gong:
> dma_terminate_all maybe called in interrupt context which means
> WARN_ON() will be triggered as below when bd memory freed. Allocat
> bd memory from dma pool instead.
While the dma_pool usage works around this WARN_ON, it doesn't fix the
underlying issue. Freeing of the descriptor memory should only happen
after the SDMA is not accessing it anymore. The comment in
sdma_disable_channel_with_delay() suggests that we need to wait some
time for this to be safe.
So rather than using a dma_pool for this memory, setting up a delayed
worker to clean up the channel would work much better and even allow to
drop that nasty mdelay() from the terminate callback. Then
device_synchronize function needs to be implemented properly, but
that's much better as than the blocking wait in atomic context.
> [ 29.161079] WARNING: CPU: 1 PID: 533 at ./include/linux/dma-mapping.h:541 sdma_free_bd+0xa4/0xb4
> [ 29.169883] Modules linked in:
> [ 29.172990] CPU: 1 PID: 533 Comm: mpegaudioparse0 Not tainted 4.18.0-rc1-next-20180618-00009-gf79f22c #20
> [ 29.182597] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
> [ 29.189163] Backtrace:
> [ 29.191685] [<c010d1e0>] (dump_backtrace) from [<c010d4a0>] (show_stack+0x18/0x1c)
> [ 29.199306] r7:00000000 r6:600f0093 r5:00000000 r4:c107db7c
> [ 29.205029] [<c010d488>] (show_stack) from [<c0a5bba0>] (dump_stack+0xb4/0xe8)
> [ 29.212312] [<c0a5baec>] (dump_stack) from [<c012703c>] (__warn+0x104/0x130)
> [ 29.219411] r9:ec3e817c r8:0000021d r7:00000009 r6:c0d1d440 r5:00000000 r4:00000000
> [ 29.227204] [<c0126f38>] (__warn) from [<c0127180>] (warn_slowpath_null+0x44/0x50)
> [ 29.234821] r8:ed129dc4 r7:c0b01978 r6:c04d4e90 r5:0000021d r4:c0d1d440
> [ 29.241574] [<c012713c>] (warn_slowpath_null) from [<c04d4e90>] (sdma_free_bd+0xa4/0xb4)
> [ 29.249706] r6:4c001000 r5:f082e000 r4:00000024
> [ 29.254376] [<c04d4dec>] (sdma_free_bd) from [<c04d4eb4>] (sdma_desc_free+0x14/0x20)
> [ 29.262163] r7:ec3e8110 r6:00000100 r5:00000200 r4:ecf89a00
> [ 29.267873] [<c04d4ea0>] (sdma_desc_free) from [<c04d229c>] (vchan_dma_desc_free_list+0xa4/0xac)
> [ 29.276697] r5:00000200 r4:ed129d9c
> [ 29.280326] [<c04d21f8>] (vchan_dma_desc_free_list) from [<c04d482c>] (sdma_disable_channel_with_delay+0x14c/0x188)
> [ 29.290808] r9:ecae560c r8:ec3e815c r7:00000000 r6:c1008908 r5:ed129dc4 r4:ec3e8110
> [ 29.298605] [<c04d46e0>] (sdma_disable_channel_with_delay) from [<c07c5c84>] (snd_dmaengine_pcm_trigger+0x90/0x1b0)
> [ 29.309087] r8:ecae5000 r7:ec940800 r6:ed31bd80 r5:ecadb200 r4:ec26a700
> [ 29.315855] [<c07c5bf4>] (snd_dmaengine_pcm_trigger) from [<c07dd800>] (soc_pcm_trigger+0xb4/0x130)
> [ 29.324953] r8:ecae5000 r7:ec940800 r6:00000000 r5:ecadb200 r4:ec26a700
> [ 29.331716] [<c07dd74c>] (soc_pcm_trigger) from [<c07bc008>] (snd_pcm_do_stop+0x58/0x5c)
> [ 29.339859] r9:ecaed5a8 r8:ed31bdc0 r7:00000000 r6:00000001 r5:ecadb200 r4:c0b9c4d0
> [ 29.347652] [<c07bbfb0>] (snd_pcm_do_stop) from [<c07bbde8>] (snd_pcm_action_single+0x40/0x80)
> [ 29.356315] [<c07bbda8>] (snd_pcm_action_single) from [<c07bbf1c>] (snd_pcm_action+0xf4/0xfc)
> [ 29.364883] r7:00000001 r6:c0b9c4d0 r5:ecadb2d4 r4:ecadb200
> [ 29.370593] [<c07bbe28>] (snd_pcm_action) from [<c07bc8dc>] (snd_pcm_drop+0x58/0x9c)
>
> > Signed-off-by: Robin Gong <yibin.gong@nxp.com>
> ---
> drivers/dma/imx-sdma.c | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> index f8becaf..7dab7e9 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -24,6 +24,7 @@
> #include <linux/spinlock.h>
> #include <linux/device.h>
> #include <linux/dma-mapping.h>
> +#include <linux/dmapool.h>
> #include <linux/firmware.h>
> #include <linux/slab.h>
> #include <linux/platform_device.h>
> @@ -343,6 +344,7 @@ struct sdma_channel {
> > > u32 shp_addr, per_addr;
> > > enum dma_status status;
> > > struct imx_dma_data data;
> > > + struct dma_pool *bd_pool;
> };
>
> > #define IMX_DMA_SG_LOOP BIT(0)
> @@ -1153,11 +1155,10 @@ static int sdma_request_channel0(struct sdma_engine *sdma)
>
> static int sdma_alloc_bd(struct sdma_desc *desc)
> {
> > - u32 bd_size = desc->num_bd * sizeof(struct sdma_buffer_descriptor);
> > int ret = 0;
>
> > - desc->bd = dma_zalloc_coherent(NULL, bd_size, &desc->bd_phys,
> > - GFP_ATOMIC);
> > + desc->bd = dma_pool_alloc(desc->sdmac->bd_pool, GFP_ATOMIC,
> + &desc->bd_phys);
This is replacing a allocation of num_bd * sizeof(descriptor) by a
single call to dma_pool_alloc, which provides enough memory for a
single descriptor, so this will lead to invalid usage of descriptor
memory for all transfers with more than a single descriptor.
Regards,
Lucas
> if (!desc->bd) {
> > ret = -ENOMEM;
> > goto out;
> @@ -1168,9 +1169,7 @@ static int sdma_alloc_bd(struct sdma_desc *desc)
>
> static void sdma_free_bd(struct sdma_desc *desc)
> {
> > - u32 bd_size = desc->num_bd * sizeof(struct sdma_buffer_descriptor);
> -
> > - dma_free_coherent(NULL, bd_size, desc->bd, desc->bd_phys);
> > + dma_pool_free(desc->sdmac->bd_pool, desc->bd, desc->bd_phys);
> }
>
> static void sdma_desc_free(struct virt_dma_desc *vd)
> @@ -1218,6 +1217,10 @@ static int sdma_alloc_chan_resources(struct dma_chan *chan)
> > if (ret)
> > goto disable_clk_ahb;
>
> > + sdmac->bd_pool = dma_pool_create("bd_pool", chan->device->dev,
> > + sizeof(struct sdma_buffer_descriptor),
> > + 32, 0);
> +
> > return 0;
>
> disable_clk_ahb:
> @@ -1246,6 +1249,9 @@ static void sdma_free_chan_resources(struct dma_chan *chan)
>
> > clk_disable(sdma->clk_ipg);
> > clk_disable(sdma->clk_ahb);
> +
> > + dma_pool_destroy(sdmac->bd_pool);
> > + sdmac->bd_pool = NULL;
> }
>
> static struct sdma_desc *sdma_transfer_init(struct sdma_channel *sdmac,
^ permalink raw reply
* Re: [PATCH v2 08/18] serial: intel: Get serial id from dts
From: Wu, Songjun @ 2018-08-06 9:32 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: hua.ma, yixin.zhu, chuanhua.lei, qi-ming.wu, linux-mips,
linux-clk, linux-serial, devicetree, linux-kernel, Jiri Slaby
In-Reply-To: <20180803054304.GA2214@kroah.com>
On 8/3/2018 1:43 PM, Greg Kroah-Hartman wrote:
> On Fri, Aug 03, 2018 at 11:02:27AM +0800, Songjun Wu wrote:
>> Get serial id from dts.
>>
>> "#ifdef CONFIG_LANTIQ" preprocessor is used because LTQ_EARLY_ASC
>> macro is defined in lantiq_soc.h.
>> lantiq_soc.h is in arch path for legacy product support.
>>
>> arch/mips/include/asm/mach-lantiq/xway/lantiq_soc.h
>>
>> If "#ifdef preprocessor" is changed to
>> "if (IS_ENABLED(CONFIG_LANTIQ))", when CONFIG_LANTIQ is not enabled,
>> code using LTQ_EARLY_ASC is compiled.
>> Compilation will fail for no LTQ_EARLY_ASC defined.
>>
>> Signed-off-by: Songjun Wu <songjun.wu@linux.intel.com>
>> ---
>>
>> Changes in v2: None
>>
>> drivers/tty/serial/lantiq.c | 19 +++++++++++++++----
>> 1 file changed, 15 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/tty/serial/lantiq.c b/drivers/tty/serial/lantiq.c
>> index 044128277248..836ca51460f2 100644
>> --- a/drivers/tty/serial/lantiq.c
>> +++ b/drivers/tty/serial/lantiq.c
>> @@ -6,6 +6,7 @@
>> * Copyright (C) 2007 Felix Fietkau <nbd@openwrt.org>
>> * Copyright (C) 2007 John Crispin <john@phrozen.org>
>> * Copyright (C) 2010 Thomas Langer, <thomas.langer@lantiq.com>
>> + * Copyright (C) 2018 Intel Corporation.
> Your changes here do not warrent the addition of a copyright line, don't
> you agree? If not, please get a signed-off-by from your corporate
> lawyer who does this this is warrented when you resend this patch.
>
> thanks,
>
> greg k-h
>
Thanks.
The copyright line will be removed when we resend this patch.
^ permalink raw reply
* Re: [PATCH v2 14/18] serial: intel: Add CCF support
From: Geert Uytterhoeven @ 2018-08-06 9:29 UTC (permalink / raw)
To: songjun.wu
Cc: Arnd Bergmann, Greg KH, Hauke Mehrtens, hua.ma, yixin.zhu,
chuanhua.lei, qi-ming.wu, Linux MIPS Mailing List, linux-clk,
open list:SERIAL DRIVERS,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux Kernel Mailing List, Jiri Slaby
In-Reply-To: <0ab8e6e7-3cc2-8e50-b1f3-99616437f527@linux.intel.com>
Hi Songjun,
On Mon, Aug 6, 2018 at 10:58 AM Wu, Songjun <songjun.wu@linux.intel.com> wrote:
> On 8/6/2018 3:20 PM, Geert Uytterhoeven wrote:
> > On Mon, Aug 6, 2018 at 9:15 AM Wu, Songjun <songjun.wu@linux.intel.com> wrote:
> >> On 8/5/2018 5:03 AM, Arnd Bergmann wrote:
> >>> On Sat, Aug 4, 2018 at 2:43 PM, Greg Kroah-Hartman
> >>> <gregkh@linuxfoundation.org> wrote:
> >>>> On Sat, Aug 04, 2018 at 12:54:22PM +0200, Hauke Mehrtens wrote:
> >>>>> On 08/03/2018 12:30 PM, Greg Kroah-Hartman wrote:
> >>>>>> On Fri, Aug 03, 2018 at 03:33:38PM +0800, Wu, Songjun wrote:
> >>>>> This patch makes it possible to use it with the legacy lantiq code and
> >>>>> also with the common clock framework. I see multiple options to fix this
> >>>>> problem.
> >>>>>
> >>>>> 1. The current approach to have it as a compile variant for a) legacy
> >>>>> lantiq arch code without common clock framework and b) support for SoCs
> >>>>> using the common clock framework.
> >>>>> 2. Convert the lantiq arch code to the common clock framework. This
> >>>>> would be a good approach, but it need some efforts.
> >>>>> 3. Remove the arch/mips/lantiq code. There are still users of this code.
> >>>>> 4. Use the old APIs also for the new xRX500 SoC, I do not like this
> >>>>> approach.
> >>>>> 5. Move lantiq_soc.h to somewhere in include/linux/ so it is globally
> >>>>> available and provide some better wrapper code.
> >>>> I don't really care what you do at this point in time, but you all
> >>>> should know better than the crazy #ifdef is not allowed to try to
> >>>> prevent/allow the inclusion of a .h file. Checkpatch might have even
> >>>> warned you about it, right?
> >>>>
> >>>> So do it correctly, odds are #5 is correct, as that makes it work like
> >>>> any other device in the kernel. You are not unique here.
> >>> The best approach here would clearly be 2. We don't want platform
> >>> specific header files for doing things that should be completely generic.
> >>>
> >>> Converting lantiq to the common-clk framework obviously requires
> >>> some work, but then again the whole arch/mips/lantiq/clk.c file
> >>> is fairly short and maybe not that hard to convert.
> >>>
> >>> >From looking at arch/mips/lantiq/xway/sysctrl.c, it appears that you
> >>> already use the clkdev lookup mechanism for some devices without
> >>> using COMMON_CLK, so I would assume that you can also use those
> >>> for the remaining clks, which would be much simpler. It registers
> >>> one anonymous clk there as
> >>>
> >>> clkdev_add_pmu("1e100c00.serial", NULL, 0, 0, PMU_ASC1);
> >>>
> >>> so why not add replace that with two named clocks and just use
> >>> the same names in the DT for the newer chip?
> >>>
> >>> Arnd
> >> We discussed internally and have another solution for this issue.
> >> Add one lantiq.h in the serial folder, and use "#ifdef preprocessor" in
> >> lantiq.h,
> >> also providing no-op stub functions in the #else case, then call those
> >> functions
> >> unconditionally from lantiq.c to avoid #ifdef in C file.
> >>
> >> To support CCF in legacy product is another topic, is not included in
> >> this patch.
> >>
> >> The implementation is as following：
> >> #ifdef CONFIG_LANTIQ
> >> #include <lantiq_soc.h>
> >> #else
> >> #define LTQ_EARLY_ASC 0
> >> #define CPHYSADDR(_val) 0
> >>
> >> static inline struct clk *clk_get_fpi(void)
> >> {
> >> return NULL;
> >> }
> >> #endif
> > Why not use clkdev_add(), as Arnd suggested?
> > That would be a 3-line patch without introducing a new header file and an ugly
> > #ifdef, which complicates compile coverage testing?
> >
> The reason we add a new head file is also for two macros(LTQ_EARLY_ASC
> and CPHYSADDR)
> used by legacy product. We need to provide the no-op stub for these two
> macro for new product.
No you don't. The line number should not be obtained by comparing the
resource address with a hardcoded base address.
Perhaps the override of port->line should just be removed, as IIRC, the serial
core has already filled in that field with the (next available) line number?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox