* [PATCH v2 0/3] Support for Apple SoCs' NCO blocks @ 2022-01-18 19:20 Martin Povišer 2022-01-18 19:21 ` [PATCH v2 1/3] dt-bindings: clock: Add Apple NCO Martin Povišer ` (2 more replies) 0 siblings, 3 replies; 10+ messages in thread From: Martin Povišer @ 2022-01-18 19:20 UTC (permalink / raw) To: mturquette, sboyd, robh+dt Cc: marcan, sven, alyssa, linux-clk, devicetree, linux-kernel, kettenis, Martin Povišer Hi, this is v2 of the common clock driver for NCO blocks on recent Apple SoCs. Changes since v1: - drop apple,nchannels property from the binding, rely on size of the register range instead to gauge the number of channels - add a lock to guard set_rate from racing with clock disable/enable - add short "theory of operation" comment - incorporate minor changes from Rob's, Sven's and Stephen's review (thanks!) Martin v1: https://lore.kernel.org/linux-clk/20211214120213.15649-1-povik@protonmail.com/ Martin Povišer (3): dt-bindings: clock: Add Apple NCO clk: clk-apple-nco: Add driver for Apple NCO MAINTAINERS: Add clk-apple-nco under ARM/APPLE MACHINE .../devicetree/bindings/clock/apple,nco.yaml | 62 ++++ MAINTAINERS | 1 + drivers/clk/Kconfig | 9 + drivers/clk/Makefile | 1 + drivers/clk/clk-apple-nco.c | 340 ++++++++++++++++++ 5 files changed, 413 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/apple,nco.yaml create mode 100644 drivers/clk/clk-apple-nco.c -- 2.33.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 1/3] dt-bindings: clock: Add Apple NCO 2022-01-18 19:20 [PATCH v2 0/3] Support for Apple SoCs' NCO blocks Martin Povišer @ 2022-01-18 19:21 ` Martin Povišer 2022-01-18 22:00 ` Mark Kettenis ` (2 more replies) 2022-01-18 19:21 ` [PATCH v2 2/3] clk: clk-apple-nco: Add driver for " Martin Povišer 2022-01-18 19:21 ` [PATCH v2 3/3] MAINTAINERS: Add clk-apple-nco under ARM/APPLE MACHINE Martin Povišer 2 siblings, 3 replies; 10+ messages in thread From: Martin Povišer @ 2022-01-18 19:21 UTC (permalink / raw) To: mturquette, sboyd, robh+dt Cc: marcan, sven, alyssa, linux-clk, devicetree, linux-kernel, kettenis, Martin Povišer The NCO block found on Apple SoCs is a programmable clock generator performing fractional division of a high frequency input clock. Signed-off-by: Martin Povišer <povik+lin@protonmail.com> --- .../devicetree/bindings/clock/apple,nco.yaml | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/apple,nco.yaml diff --git a/Documentation/devicetree/bindings/clock/apple,nco.yaml b/Documentation/devicetree/bindings/clock/apple,nco.yaml new file mode 100644 index 000000000000..da56b64b8fff --- /dev/null +++ b/Documentation/devicetree/bindings/clock/apple,nco.yaml @@ -0,0 +1,62 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/clock/apple,nco.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Apple SoCs' NCO block + +maintainers: + - Martin Povišer <povik+lin@protonmail.com> + +description: | + The NCO (Numerically Controlled Oscillator) block found on Apple SoCs + such as the t8103 (M1) is a programmable clock generator performing + fractional division of a high frequency input clock. + + It carries a number of independent channels and is typically used for + generation of audio bitclocks. + +properties: + compatible: + items: + - enum: + - apple,t6000-nco + - apple,t8103-nco + - const: apple,nco + + clocks: + description: + Specifies the reference clock from which the output clocks + are derived through fractional division. + maxItems: 1 + + '#clock-cells': + const: 1 + + reg: + maxItems: 1 + +required: + - compatible + - clocks + - '#clock-cells' + - reg + +additionalProperties: false + +examples: + - | + nco_clkref: clock-ref { + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <900000000>; + clock-output-names = "nco-ref"; + }; + + nco: clock-controller@23b044000 { + compatible = "apple,t8103-nco", "apple,nco"; + reg = <0x3b044000 0x14000>; + #clock-cells = <1>; + clocks = <&nco_clkref>; + }; -- 2.33.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/3] dt-bindings: clock: Add Apple NCO 2022-01-18 19:21 ` [PATCH v2 1/3] dt-bindings: clock: Add Apple NCO Martin Povišer @ 2022-01-18 22:00 ` Mark Kettenis 2022-01-19 2:55 ` Rob Herring 2022-01-19 14:01 ` Rob Herring 2 siblings, 0 replies; 10+ messages in thread From: Mark Kettenis @ 2022-01-18 22:00 UTC (permalink / raw) To: Martin Povišer Cc: mturquette, sboyd, robh+dt, marcan, sven, alyssa, linux-clk, devicetree, linux-kernel, kettenis, povik+lin > Date: Tue, 18 Jan 2022 19:21:03 +0000 > From: Martin Povišer <povik+lin@protonmail.com> > > The NCO block found on Apple SoCs is a programmable clock generator > performing fractional division of a high frequency input clock. > > Signed-off-by: Martin Povišer <povik+lin@protonmail.com> Reviewed-by: Mark Kettenis <kettenis@openbsd.org> > --- > .../devicetree/bindings/clock/apple,nco.yaml | 62 +++++++++++++++++++ > 1 file changed, 62 insertions(+) > create mode 100644 Documentation/devicetree/bindings/clock/apple,nco.yaml > > diff --git a/Documentation/devicetree/bindings/clock/apple,nco.yaml b/Documentation/devicetree/bindings/clock/apple,nco.yaml > new file mode 100644 > index 000000000000..da56b64b8fff > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/apple,nco.yaml > @@ -0,0 +1,62 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/clock/apple,nco.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Apple SoCs' NCO block > + > +maintainers: > + - Martin Povišer <povik+lin@protonmail.com> > + > +description: | > + The NCO (Numerically Controlled Oscillator) block found on Apple SoCs > + such as the t8103 (M1) is a programmable clock generator performing > + fractional division of a high frequency input clock. > + > + It carries a number of independent channels and is typically used for > + generation of audio bitclocks. > + > +properties: > + compatible: > + items: > + - enum: > + - apple,t6000-nco > + - apple,t8103-nco > + - const: apple,nco > + > + clocks: > + description: > + Specifies the reference clock from which the output clocks > + are derived through fractional division. > + maxItems: 1 > + > + '#clock-cells': > + const: 1 > + > + reg: > + maxItems: 1 > + > +required: > + - compatible > + - clocks > + - '#clock-cells' > + - reg > + > +additionalProperties: false > + > +examples: > + - | > + nco_clkref: clock-ref { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <900000000>; > + clock-output-names = "nco-ref"; > + }; > + > + nco: clock-controller@23b044000 { > + compatible = "apple,t8103-nco", "apple,nco"; > + reg = <0x3b044000 0x14000>; > + #clock-cells = <1>; > + clocks = <&nco_clkref>; > + }; > -- > 2.33.0 > > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/3] dt-bindings: clock: Add Apple NCO 2022-01-18 19:21 ` [PATCH v2 1/3] dt-bindings: clock: Add Apple NCO Martin Povišer 2022-01-18 22:00 ` Mark Kettenis @ 2022-01-19 2:55 ` Rob Herring 2022-01-19 14:01 ` Rob Herring 2 siblings, 0 replies; 10+ messages in thread From: Rob Herring @ 2022-01-19 2:55 UTC (permalink / raw) To: Martin Povišer Cc: robh+dt, linux-kernel, alyssa, mturquette, sboyd, linux-clk, kettenis, sven, marcan, devicetree [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain, Size: 1180 bytes --] On Tue, 18 Jan 2022 19:21:03 +0000, Martin Povišer wrote: > The NCO block found on Apple SoCs is a programmable clock generator > performing fractional division of a high frequency input clock. > > Signed-off-by: Martin Povišer <povik+lin@protonmail.com> > --- > .../devicetree/bindings/clock/apple,nco.yaml | 62 +++++++++++++++++++ > 1 file changed, 62 insertions(+) > create mode 100644 Documentation/devicetree/bindings/clock/apple,nco.yaml > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: ./Documentation/devicetree/bindings/clock/apple,nco.yaml:24:9: [warning] wrong indentation: expected 10 but found 8 (indentation) dtschema/dtc warnings/errors: doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/patch/1581480 This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/3] dt-bindings: clock: Add Apple NCO 2022-01-18 19:21 ` [PATCH v2 1/3] dt-bindings: clock: Add Apple NCO Martin Povišer 2022-01-18 22:00 ` Mark Kettenis 2022-01-19 2:55 ` Rob Herring @ 2022-01-19 14:01 ` Rob Herring 2 siblings, 0 replies; 10+ messages in thread From: Rob Herring @ 2022-01-19 14:01 UTC (permalink / raw) To: Martin Povišer Cc: mturquette, sboyd, marcan, sven, alyssa, linux-clk, devicetree, linux-kernel, kettenis On Tue, Jan 18, 2022 at 07:21:03PM +0000, Martin Povišer wrote: > The NCO block found on Apple SoCs is a programmable clock generator > performing fractional division of a high frequency input clock. > > Signed-off-by: Martin Povišer <povik+lin@protonmail.com> > --- > .../devicetree/bindings/clock/apple,nco.yaml | 62 +++++++++++++++++++ > 1 file changed, 62 insertions(+) > create mode 100644 Documentation/devicetree/bindings/clock/apple,nco.yaml With the indentation fix, Reviewed-by: Rob Herring <robh@kernel.org> ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 2/3] clk: clk-apple-nco: Add driver for Apple NCO 2022-01-18 19:20 [PATCH v2 0/3] Support for Apple SoCs' NCO blocks Martin Povišer 2022-01-18 19:21 ` [PATCH v2 1/3] dt-bindings: clock: Add Apple NCO Martin Povišer @ 2022-01-18 19:21 ` Martin Povišer 2022-01-20 5:38 ` Stephen Boyd 2022-01-18 19:21 ` [PATCH v2 3/3] MAINTAINERS: Add clk-apple-nco under ARM/APPLE MACHINE Martin Povišer 2 siblings, 1 reply; 10+ messages in thread From: Martin Povišer @ 2022-01-18 19:21 UTC (permalink / raw) To: mturquette, sboyd, robh+dt Cc: marcan, sven, alyssa, linux-clk, devicetree, linux-kernel, kettenis, Martin Povišer Add a common clock driver for NCO blocks found on Apple SoCs where they are typically the generators of audio clocks. Signed-off-by: Martin Povišer <povik+lin@protonmail.com> --- drivers/clk/Kconfig | 9 + drivers/clk/Makefile | 1 + drivers/clk/clk-apple-nco.c | 340 ++++++++++++++++++++++++++++++++++++ 3 files changed, 350 insertions(+) create mode 100644 drivers/clk/clk-apple-nco.c diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig index ad4256d54361..af4d037e18e3 100644 --- a/drivers/clk/Kconfig +++ b/drivers/clk/Kconfig @@ -59,6 +59,15 @@ config LMK04832 Say yes here to build support for Texas Instruments' LMK04832 Ultra Low-Noise JESD204B Compliant Clock Jitter Cleaner With Dual Loop PLLs +config COMMON_CLK_APPLE_NCO + bool "Clock driver for Apple SoC NCOs" + depends on ARCH_APPLE || COMPILE_TEST + default ARCH_APPLE + help + This driver supports NCO (Numerically Controlled Oscillator) blocks + found on Apple SoCs such as t8103 (M1). The blocks are typically + generators of audio clocks. + config COMMON_CLK_MAX77686 tristate "Clock driver for Maxim 77620/77686/77802 MFD" depends on MFD_MAX77686 || MFD_MAX77620 || COMPILE_TEST diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile index 16e588630472..e95e702bdaeb 100644 --- a/drivers/clk/Makefile +++ b/drivers/clk/Makefile @@ -17,6 +17,7 @@ endif # hardware specific clock types # please keep this section sorted lexicographically by file path name +obj-$(CONFIG_COMMON_CLK_APPLE_NCO) += clk-apple-nco.o obj-$(CONFIG_MACH_ASM9260) += clk-asm9260.o obj-$(CONFIG_COMMON_CLK_AXI_CLKGEN) += clk-axi-clkgen.o obj-$(CONFIG_ARCH_AXXIA) += clk-axm5516.o diff --git a/drivers/clk/clk-apple-nco.c b/drivers/clk/clk-apple-nco.c new file mode 100644 index 000000000000..593f5b5ce5b7 --- /dev/null +++ b/drivers/clk/clk-apple-nco.c @@ -0,0 +1,340 @@ +// SPDX-License-Identifier: GPL-2.0-only OR MIT +/* + * Driver for an SoC block (Numerically Controlled Oscillator) + * found on t8103 (M1) and other Apple chips + * + * Copyright (C) The Asahi Linux Contributors + */ + +#include <linux/bits.h> +#include <linux/clk-provider.h> +#include <linux/io.h> +#include <linux/math64.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_clk.h> +#include <linux/platform_device.h> +#include <linux/spinlock.h> + +#define NCO_CHANNEL_STRIDE 0x4000 +#define NCO_CHANNEL_REGSIZE 20 + +#define REG_CTRL 0 +#define CTRL_ENABLE BIT(31) +#define REG_DIV 4 +#define DIV_FINE GENMASK(1, 0) +#define DIV_COARSE GENMASK(12, 2) +#define REG_INC1 8 +#define REG_INC2 12 +#define REG_ACCINIT 16 + +/* + * Theory of operation (postulated) + * + * The REG_DIV register indirectly expresses a base integer divisor, roughly + * corresponding to twice the desired ratio of input to output clock. This + * base divisor is adjusted on a cycle-by-cycle basis based on the state of a + * 32-bit phase accumulator to achieve a desired precise clock ratio over the + * long term. + * + * Specifically an output clock cycle is produced after (REG_DIV divisor)/2 + * or (REG_DIV divisor + 1)/2 input cycles, the latter taking effect when top + * bit of the 32-bit accumulator is set. The accumulator is incremented each + * produced output cycle, by the value from either REG_INC1 or REG_INC2, which + * of the two is selected depending again on the accumulator's current top bit. + * + * Because the NCO hardware implements counting of input clock cycles in part + * in a Galois linear-feedback shift register, the higher bits of divisor + * are programmed into REG_DIV by picking an appropriate LFSR state. See + * nco_compute_tables/nco_div_translate for details on this. + */ + +struct nco_tables; + +struct nco_channel { + void __iomem *base; + struct nco_tables *tbl; + struct clk_hw hw; + + spinlock_t lock; +}; + +#define to_nco_channel(_hw) container_of(_hw, struct nco_channel, hw) + +#define LFSR_POLY 0xa01 +#define LFSR_INIT 0x7ff +#define LFSR_LEN 11 +#define LFSR_PERIOD ((1 << LFSR_LEN) - 1) +#define LFSR_TBLSIZE (1 << LFSR_LEN) + +/* The minimal attainable coarse divisor (first value in table) */ +#define COARSE_DIV_OFFSET 2 + +struct nco_tables { + u16 fwd[LFSR_TBLSIZE]; + u16 inv[LFSR_TBLSIZE]; +}; + +static void nco_enable_nolock(struct clk_hw *hw); +static void nco_disable_nolock(struct clk_hw *hw); +static int nco_is_enabled(struct clk_hw *hw); + +static void nco_compute_tables(struct nco_tables *tbl) +{ + int i; + u32 state = LFSR_INIT; + + /* + * Go through the states of a Galois LFSR and build + * a coarse divisor translation table. + */ + for (i = LFSR_PERIOD; i > 0; i--) { + if (state & 1) + state = (state >> 1) ^ (LFSR_POLY >> 1); + else + state = (state >> 1); + tbl->fwd[i] = state; + tbl->inv[state] = i; + } + + /* Zero value is special-cased */ + tbl->fwd[0] = 0; + tbl->inv[0] = 0; +} + +static bool nco_div_out_of_range(unsigned int div) +{ + unsigned int coarse = div / 4; + return coarse < COARSE_DIV_OFFSET || + coarse >= COARSE_DIV_OFFSET + LFSR_TBLSIZE; +} + +static u32 nco_div_translate(struct nco_tables *tbl, unsigned int div) +{ + unsigned int coarse = div / 4; + + if (WARN_ON(nco_div_out_of_range(div))) + return 0; + + return FIELD_PREP(DIV_COARSE, tbl->fwd[coarse - COARSE_DIV_OFFSET]) | + FIELD_PREP(DIV_FINE, div % 4); +} + +static unsigned int nco_div_translate_inv(struct nco_tables *tbl, u32 regval) +{ + unsigned int coarse, fine; + + coarse = tbl->inv[FIELD_GET(DIV_COARSE, regval)] + COARSE_DIV_OFFSET; + fine = FIELD_GET(DIV_FINE, regval); + + return coarse * 4 + fine; +} + +static int nco_set_rate(struct clk_hw *hw, unsigned long rate, + unsigned long parent_rate) +{ + struct nco_channel *chan = to_nco_channel(hw); + unsigned long flags; + u32 div; + s32 inc1, inc2; + bool was_enabled; + + div = 2 * parent_rate / rate; + inc1 = 2 * parent_rate - div * rate; + inc2 = -((s32) (rate - inc1)); + + if (nco_div_out_of_range(div)) + return -EINVAL; + + div = nco_div_translate(chan->tbl, div); + + spin_lock_irqsave(&chan->lock, flags); + was_enabled = nco_is_enabled(hw); + nco_disable_nolock(hw); + + writel_relaxed(div, chan->base + REG_DIV); + writel_relaxed(inc1, chan->base + REG_INC1); + writel_relaxed(inc2, chan->base + REG_INC2); + + /* Presumably a neutral initial value for accumulator */ + writel_relaxed(1 << 31, chan->base + REG_ACCINIT); + + if (was_enabled) + nco_enable_nolock(hw); + spin_unlock_irqrestore(&chan->lock, flags); + + return 0; +} + +static unsigned long nco_recalc_rate(struct clk_hw *hw, + unsigned long parent_rate) +{ + struct nco_channel *chan = to_nco_channel(hw); + u32 div; + s32 inc1, inc2, incbase; + + div = nco_div_translate_inv(chan->tbl, + readl_relaxed(chan->base + REG_DIV)); + + inc1 = readl_relaxed(chan->base + REG_INC1); + inc2 = readl_relaxed(chan->base + REG_INC2); + + /* + * We don't support wraparound of accumulator + * nor the edge case of both increments being zero + */ + if (inc1 < 0 || inc2 > 0 || (inc1 == 0 && inc2 == 0)) + return 0; + + /* Scale both sides of division by incbase to maintain precision */ + incbase = inc1 - inc2; + + return div_u64(((u64) parent_rate) * 2 * incbase, + ((u64) div) * incbase + inc1); +} + +static long nco_round_rate(struct clk_hw *hw, unsigned long rate, + unsigned long *parent_rate) +{ + unsigned long lo = *parent_rate / (COARSE_DIV_OFFSET + LFSR_TBLSIZE) + 1; + unsigned long hi = *parent_rate / COARSE_DIV_OFFSET; + + return clamp(rate, lo, hi); +} + +static void nco_enable_nolock(struct clk_hw *hw) +{ + struct nco_channel *chan = to_nco_channel(hw); + u32 val; + + val = readl_relaxed(chan->base + REG_CTRL); + writel_relaxed(val | CTRL_ENABLE, chan->base + REG_CTRL); +} + +static void nco_disable_nolock(struct clk_hw *hw) +{ + struct nco_channel *chan = to_nco_channel(hw); + u32 val; + + val = readl_relaxed(chan->base + REG_CTRL); + writel_relaxed(val & ~CTRL_ENABLE, chan->base + REG_CTRL); +} + +static int nco_is_enabled(struct clk_hw *hw) +{ + struct nco_channel *chan = to_nco_channel(hw); + + return (readl_relaxed(chan->base + REG_CTRL) & CTRL_ENABLE) != 0; +} + +static int nco_enable(struct clk_hw *hw) +{ + struct nco_channel *chan = to_nco_channel(hw); + unsigned long flags; + + spin_lock_irqsave(&chan->lock, flags); + nco_enable_nolock(hw); + spin_unlock_irqrestore(&chan->lock, flags); + + return 0; +} + +static void nco_disable(struct clk_hw *hw) +{ + struct nco_channel *chan = to_nco_channel(hw); + unsigned long flags; + + spin_lock_irqsave(&chan->lock, flags); + nco_disable_nolock(hw); + spin_unlock_irqrestore(&chan->lock, flags); +} + +static const struct clk_ops nco_ops = { + .set_rate = nco_set_rate, + .recalc_rate = nco_recalc_rate, + .round_rate = nco_round_rate, + .enable = nco_enable, + .disable = nco_disable, + .is_enabled = nco_is_enabled, +}; + +static int apple_nco_probe(struct platform_device *pdev) +{ + struct device_node *np = pdev->dev.of_node; + struct clk_parent_data pdata = { .index = 0 }; + struct clk_init_data init; + struct clk_hw_onecell_data *onecell_data; + void __iomem *regs; + struct resource *regs_res; + struct nco_tables *tbl; + unsigned int nchannels; + int ret, i; + + regs = devm_platform_get_and_ioremap_resource(pdev, 0, ®s_res); + if (IS_ERR(regs)) + return PTR_ERR(regs); + + if (resource_size(regs_res) < NCO_CHANNEL_REGSIZE) + return -EINVAL; + nchannels = (resource_size(regs_res) - NCO_CHANNEL_REGSIZE) + / NCO_CHANNEL_STRIDE + 1; + + onecell_data = devm_kzalloc(&pdev->dev, struct_size(onecell_data, hws, + nchannels), GFP_KERNEL); + if (!onecell_data) + return -ENOMEM; + onecell_data->num = nchannels; + + tbl = devm_kzalloc(&pdev->dev, sizeof(*tbl), GFP_KERNEL); + if (!tbl) + return -ENOMEM; + nco_compute_tables(tbl); + + for (i = 0; i < nchannels; i++) { + struct nco_channel *chan; + + chan = devm_kzalloc(&pdev->dev, sizeof(*chan), GFP_KERNEL); + if (!chan) + return -ENOMEM; + chan->base = regs + NCO_CHANNEL_STRIDE*i; + chan->tbl = tbl; + spin_lock_init(&chan->lock); + + memset(&init, 0, sizeof(init)); + init.name = devm_kasprintf(&pdev->dev, GFP_KERNEL, + "%s-%d", np->name, i); + init.ops = &nco_ops; + init.parent_data = &pdata; + init.num_parents = 1; + init.flags = 0; + + chan->hw.init = &init; + ret = devm_clk_hw_register(&pdev->dev, &chan->hw); + if (ret) + return ret; + + onecell_data->hws[i] = &chan->hw; + } + + return devm_of_clk_add_hw_provider(&pdev->dev, of_clk_hw_onecell_get, + onecell_data); +} + +static const struct of_device_id apple_nco_ids[] = { + { .compatible = "apple,nco" }, + { } +}; +MODULE_DEVICE_TABLE(of, apple_nco_ids) + +static struct platform_driver apple_nco_driver = { + .driver = { + .name = "apple-nco", + .of_match_table = apple_nco_ids, + }, + .probe = apple_nco_probe, +}; +module_platform_driver(apple_nco_driver); + +MODULE_AUTHOR("Martin Povišer <povik+lin@protonmail.com>"); +MODULE_DESCRIPTION("Clock driver for NCO blocks on Apple SoCs"); +MODULE_LICENSE("GPL v2"); -- 2.33.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/3] clk: clk-apple-nco: Add driver for Apple NCO 2022-01-18 19:21 ` [PATCH v2 2/3] clk: clk-apple-nco: Add driver for " Martin Povišer @ 2022-01-20 5:38 ` Stephen Boyd 2022-01-20 12:11 ` Martin Povišer 0 siblings, 1 reply; 10+ messages in thread From: Stephen Boyd @ 2022-01-20 5:38 UTC (permalink / raw) To: Martin Povišer, mturquette, robh+dt Cc: marcan, sven, alyssa, linux-clk, devicetree, linux-kernel, kettenis, Martin Povišer Quoting Martin Povišer (2022-01-18 11:21:10) > diff --git a/drivers/clk/clk-apple-nco.c b/drivers/clk/clk-apple-nco.c > new file mode 100644 > index 000000000000..593f5b5ce5b7 > --- /dev/null > +++ b/drivers/clk/clk-apple-nco.c > @@ -0,0 +1,340 @@ > +// SPDX-License-Identifier: GPL-2.0-only OR MIT > +/* > + * Driver for an SoC block (Numerically Controlled Oscillator) > + * found on t8103 (M1) and other Apple chips > + * > + * Copyright (C) The Asahi Linux Contributors > + */ > + > +#include <linux/bits.h> > +#include <linux/clk-provider.h> > +#include <linux/io.h> > +#include <linux/math64.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/of_clk.h> Is this include used? If not, please drop it. Please include kernel.h for container_of() usage. > +#include <linux/platform_device.h> > +#include <linux/spinlock.h> > + > +#define NCO_CHANNEL_STRIDE 0x4000 > +#define NCO_CHANNEL_REGSIZE 20 > + > +#define REG_CTRL 0 > +#define CTRL_ENABLE BIT(31) > +#define REG_DIV 4 > +#define DIV_FINE GENMASK(1, 0) > +#define DIV_COARSE GENMASK(12, 2) > +#define REG_INC1 8 > +#define REG_INC2 12 > +#define REG_ACCINIT 16 > + > +/* > + * Theory of operation (postulated) > + * > + * The REG_DIV register indirectly expresses a base integer divisor, roughly > + * corresponding to twice the desired ratio of input to output clock. This > + * base divisor is adjusted on a cycle-by-cycle basis based on the state of a > + * 32-bit phase accumulator to achieve a desired precise clock ratio over the > + * long term. > + * > + * Specifically an output clock cycle is produced after (REG_DIV divisor)/2 > + * or (REG_DIV divisor + 1)/2 input cycles, the latter taking effect when top > + * bit of the 32-bit accumulator is set. The accumulator is incremented each > + * produced output cycle, by the value from either REG_INC1 or REG_INC2, which > + * of the two is selected depending again on the accumulator's current top bit. > + * > + * Because the NCO hardware implements counting of input clock cycles in part > + * in a Galois linear-feedback shift register, the higher bits of divisor > + * are programmed into REG_DIV by picking an appropriate LFSR state. See > + * nco_compute_tables/nco_div_translate for details on this. > + */ > + > +struct nco_tables; Please declare the struct here. > + > +struct nco_channel { > + void __iomem *base; > + struct nco_tables *tbl; > + struct clk_hw hw; > + > + spinlock_t lock; > +}; > + > +#define to_nco_channel(_hw) container_of(_hw, struct nco_channel, hw) > + > +#define LFSR_POLY 0xa01 > +#define LFSR_INIT 0x7ff > +#define LFSR_LEN 11 > +#define LFSR_PERIOD ((1 << LFSR_LEN) - 1) > +#define LFSR_TBLSIZE (1 << LFSR_LEN) > + > +/* The minimal attainable coarse divisor (first value in table) */ > +#define COARSE_DIV_OFFSET 2 > + > +struct nco_tables { > + u16 fwd[LFSR_TBLSIZE]; > + u16 inv[LFSR_TBLSIZE]; > +}; Or put struct nco_channel here. > + > +static void nco_enable_nolock(struct clk_hw *hw); > +static void nco_disable_nolock(struct clk_hw *hw); > +static int nco_is_enabled(struct clk_hw *hw); Define the functions here so we don't need forward declarations. > + > +static void nco_compute_tables(struct nco_tables *tbl) > +{ > + int i; > + u32 state = LFSR_INIT; > + > + /* > + * Go through the states of a Galois LFSR and build > + * a coarse divisor translation table. > + */ > + for (i = LFSR_PERIOD; i > 0; i--) { > + if (state & 1) > + state = (state >> 1) ^ (LFSR_POLY >> 1); > + else > + state = (state >> 1); > + tbl->fwd[i] = state; > + tbl->inv[state] = i; > + } > + > + /* Zero value is special-cased */ > + tbl->fwd[0] = 0; > + tbl->inv[0] = 0; > +} > + > +static bool nco_div_out_of_range(unsigned int div) > +{ > + unsigned int coarse = div / 4; Nitpick: Newline here > + return coarse < COARSE_DIV_OFFSET || > + coarse >= COARSE_DIV_OFFSET + LFSR_TBLSIZE; > +} > + > +static u32 nco_div_translate(struct nco_tables *tbl, unsigned int div) > +{ > + unsigned int coarse = div / 4; > + > + if (WARN_ON(nco_div_out_of_range(div))) Maybe worth knowing which clk is out of range? > + return 0; > + > + return FIELD_PREP(DIV_COARSE, tbl->fwd[coarse - COARSE_DIV_OFFSET]) | > + FIELD_PREP(DIV_FINE, div % 4); > +} > + > +static unsigned int nco_div_translate_inv(struct nco_tables *tbl, u32 regval) > +{ > + unsigned int coarse, fine; > + > + coarse = tbl->inv[FIELD_GET(DIV_COARSE, regval)] + COARSE_DIV_OFFSET; > + fine = FIELD_GET(DIV_FINE, regval); > + > + return coarse * 4 + fine; > +} > + > +static int nco_set_rate(struct clk_hw *hw, unsigned long rate, > + unsigned long parent_rate) > +{ > + struct nco_channel *chan = to_nco_channel(hw); > + unsigned long flags; > + u32 div; > + s32 inc1, inc2; > + bool was_enabled; > + > + div = 2 * parent_rate / rate; > + inc1 = 2 * parent_rate - div * rate; > + inc2 = -((s32) (rate - inc1)); Is the cast necessary? > + > + if (nco_div_out_of_range(div)) > + return -EINVAL; > + > + div = nco_div_translate(chan->tbl, div); > + > + spin_lock_irqsave(&chan->lock, flags); > + was_enabled = nco_is_enabled(hw); > + nco_disable_nolock(hw); > + > + writel_relaxed(div, chan->base + REG_DIV); > + writel_relaxed(inc1, chan->base + REG_INC1); > + writel_relaxed(inc2, chan->base + REG_INC2); > + > + /* Presumably a neutral initial value for accumulator */ > + writel_relaxed(1 << 31, chan->base + REG_ACCINIT); > + > + if (was_enabled) > + nco_enable_nolock(hw); > + spin_unlock_irqrestore(&chan->lock, flags); > + > + return 0; > +} > + > +static unsigned long nco_recalc_rate(struct clk_hw *hw, > + unsigned long parent_rate) > +{ > + struct nco_channel *chan = to_nco_channel(hw); > + u32 div; > + s32 inc1, inc2, incbase; > + > + div = nco_div_translate_inv(chan->tbl, > + readl_relaxed(chan->base + REG_DIV)); > + > + inc1 = readl_relaxed(chan->base + REG_INC1); > + inc2 = readl_relaxed(chan->base + REG_INC2); > + > + /* > + * We don't support wraparound of accumulator > + * nor the edge case of both increments being zero > + */ > + if (inc1 < 0 || inc2 > 0 || (inc1 == 0 && inc2 == 0)) > + return 0; > + > + /* Scale both sides of division by incbase to maintain precision */ > + incbase = inc1 - inc2; > + > + return div_u64(((u64) parent_rate) * 2 * incbase, > + ((u64) div) * incbase + inc1); Why is the divisor casted to 64 bits? div_u64() takes a u32 divisor so if it's going to overflow 32 bits we're in trouble. > +} > + > +static long nco_round_rate(struct clk_hw *hw, unsigned long rate, > + unsigned long *parent_rate) > +{ > + unsigned long lo = *parent_rate / (COARSE_DIV_OFFSET + LFSR_TBLSIZE) + 1; > + unsigned long hi = *parent_rate / COARSE_DIV_OFFSET; > + > + return clamp(rate, lo, hi); > +} > + > +static void nco_enable_nolock(struct clk_hw *hw) > +{ > + struct nco_channel *chan = to_nco_channel(hw); > + u32 val; > + > + val = readl_relaxed(chan->base + REG_CTRL); > + writel_relaxed(val | CTRL_ENABLE, chan->base + REG_CTRL); > +} > + > +static void nco_disable_nolock(struct clk_hw *hw) > +{ > + struct nco_channel *chan = to_nco_channel(hw); > + u32 val; > + > + val = readl_relaxed(chan->base + REG_CTRL); > + writel_relaxed(val & ~CTRL_ENABLE, chan->base + REG_CTRL); > +} > + > +static int nco_is_enabled(struct clk_hw *hw) > +{ > + struct nco_channel *chan = to_nco_channel(hw); > + > + return (readl_relaxed(chan->base + REG_CTRL) & CTRL_ENABLE) != 0; > +} > + > +static int nco_enable(struct clk_hw *hw) > +{ > + struct nco_channel *chan = to_nco_channel(hw); > + unsigned long flags; > + > + spin_lock_irqsave(&chan->lock, flags); > + nco_enable_nolock(hw); > + spin_unlock_irqrestore(&chan->lock, flags); > + > + return 0; > +} > + > +static void nco_disable(struct clk_hw *hw) > +{ > + struct nco_channel *chan = to_nco_channel(hw); > + unsigned long flags; > + > + spin_lock_irqsave(&chan->lock, flags); > + nco_disable_nolock(hw); > + spin_unlock_irqrestore(&chan->lock, flags); > +} > + > +static const struct clk_ops nco_ops = { Perhaps apple_nco_ops (and apple_ prefix for the functions) so that tags in the global namespace don't conflict. > + .set_rate = nco_set_rate, > + .recalc_rate = nco_recalc_rate, > + .round_rate = nco_round_rate, > + .enable = nco_enable, > + .disable = nco_disable, > + .is_enabled = nco_is_enabled, > +}; > + > +static int apple_nco_probe(struct platform_device *pdev) > +{ > + struct device_node *np = pdev->dev.of_node; > + struct clk_parent_data pdata = { .index = 0 }; > + struct clk_init_data init; > + struct clk_hw_onecell_data *onecell_data; > + void __iomem *regs; Usually it's called 'base' > + struct resource *regs_res; Usually it's called 'res' > + struct nco_tables *tbl; > + unsigned int nchannels; > + int ret, i; > + > + regs = devm_platform_get_and_ioremap_resource(pdev, 0, ®s_res); > + if (IS_ERR(regs)) > + return PTR_ERR(regs); > + > + if (resource_size(regs_res) < NCO_CHANNEL_REGSIZE) > + return -EINVAL; > + nchannels = (resource_size(regs_res) - NCO_CHANNEL_REGSIZE) > + / NCO_CHANNEL_STRIDE + 1; Is this some sort of DIV_ROUND_UP()? > + > + onecell_data = devm_kzalloc(&pdev->dev, struct_size(onecell_data, hws, > + nchannels), GFP_KERNEL); > + if (!onecell_data) > + return -ENOMEM; > + onecell_data->num = nchannels; > + > + tbl = devm_kzalloc(&pdev->dev, sizeof(*tbl), GFP_KERNEL); > + if (!tbl) > + return -ENOMEM; > + nco_compute_tables(tbl); > + > + for (i = 0; i < nchannels; i++) { > + struct nco_channel *chan; > + > + chan = devm_kzalloc(&pdev->dev, sizeof(*chan), GFP_KERNEL); > + if (!chan) > + return -ENOMEM; > + chan->base = regs + NCO_CHANNEL_STRIDE*i; Please add space around * above. > + chan->tbl = tbl; > + spin_lock_init(&chan->lock); > + > + memset(&init, 0, sizeof(init)); > + init.name = devm_kasprintf(&pdev->dev, GFP_KERNEL, > + "%s-%d", np->name, i); > + init.ops = &nco_ops; > + init.parent_data = &pdata; > + init.num_parents = 1; > + init.flags = 0; > + > + chan->hw.init = &init; > + ret = devm_clk_hw_register(&pdev->dev, &chan->hw); > + if (ret) > + return ret; > + > + onecell_data->hws[i] = &chan->hw; > + } > + > + return devm_of_clk_add_hw_provider(&pdev->dev, of_clk_hw_onecell_get, > + onecell_data); ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/3] clk: clk-apple-nco: Add driver for Apple NCO 2022-01-20 5:38 ` Stephen Boyd @ 2022-01-20 12:11 ` Martin Povišer 2022-01-20 20:59 ` Stephen Boyd 0 siblings, 1 reply; 10+ messages in thread From: Martin Povišer @ 2022-01-20 12:11 UTC (permalink / raw) To: Stephen Boyd Cc: Martin Povišer, Michael Turquette, Rob Herring, Hector Martin, Sven Peter, alyssa, linux-clk, devicetree, linux-kernel, Mark Kettenis Hi Stephen, thanks for the review. > On 20. 1. 2022, at 6:38, Stephen Boyd <sboyd@kernel.org> wrote: > > Quoting Martin Povišer (2022-01-18 11:21:10) >> diff --git a/drivers/clk/clk-apple-nco.c b/drivers/clk/clk-apple-nco.c >> new file mode 100644 >> index 000000000000..593f5b5ce5b7 >> --- /dev/null >> +++ b/drivers/clk/clk-apple-nco.c >> +static u32 nco_div_translate(struct nco_tables *tbl, unsigned int div) >> +{ >> + unsigned int coarse = div / 4; >> + >> + if (WARN_ON(nco_div_out_of_range(div))) > > Maybe worth knowing which clk is out of range? This can only happen when nco_div_translate is used wrong, which should never happen with the code as-is, so I wouldn't think it is worth expanding on. >> + >> +static int nco_set_rate(struct clk_hw *hw, unsigned long rate, >> + unsigned long parent_rate) >> +{ >> + struct nco_channel *chan = to_nco_channel(hw); >> + unsigned long flags; >> + u32 div; >> + s32 inc1, inc2; >> + bool was_enabled; >> + >> + div = 2 * parent_rate / rate; >> + inc1 = 2 * parent_rate - div * rate; >> + inc2 = -((s32) (rate - inc1)); > > Is the cast necessary? Answering that prompted me to get back to reading some C specification and now I am confident in moving away from signed types here and in nco_recalc_rate altogether. >> >> + >> +static unsigned long nco_recalc_rate(struct clk_hw *hw, >> + unsigned long parent_rate) >> +{ >> + >> + /* Scale both sides of division by incbase to maintain precision */ >> + incbase = inc1 - inc2; >> + >> + return div_u64(((u64) parent_rate) * 2 * incbase, >> + ((u64) div) * incbase + inc1); > > Why is the divisor casted to 64 bits? div_u64() takes a u32 divisor so > if it's going to overflow 32 bits we're in trouble. Yeah, we need div64_u64, great you caught that. >> + struct nco_tables *tbl; >> + unsigned int nchannels; >> + int ret, i; >> + >> + regs = devm_platform_get_and_ioremap_resource(pdev, 0, ®s_res); >> + if (IS_ERR(regs)) >> + return PTR_ERR(regs); >> + >> + if (resource_size(regs_res) < NCO_CHANNEL_REGSIZE) >> + return -EINVAL; >> + nchannels = (resource_size(regs_res) - NCO_CHANNEL_REGSIZE) >> + / NCO_CHANNEL_STRIDE + 1; > > Is this some sort of DIV_ROUND_UP()? Almost. I will shop around for a macro replacement. Martin ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/3] clk: clk-apple-nco: Add driver for Apple NCO 2022-01-20 12:11 ` Martin Povišer @ 2022-01-20 20:59 ` Stephen Boyd 0 siblings, 0 replies; 10+ messages in thread From: Stephen Boyd @ 2022-01-20 20:59 UTC (permalink / raw) To: Martin Povišer Cc: Martin Povišer, Michael Turquette, Rob Herring, Hector Martin, Sven Peter, alyssa, linux-clk, devicetree, linux-kernel, Mark Kettenis Quoting Martin Povišer (2022-01-20 04:11:34) > > > On 20. 1. 2022, at 6:38, Stephen Boyd <sboyd@kernel.org> wrote: > > > > Quoting Martin Povišer (2022-01-18 11:21:10) > >> + > >> +static int nco_set_rate(struct clk_hw *hw, unsigned long rate, > >> + unsigned long parent_rate) > >> +{ > >> + struct nco_channel *chan = to_nco_channel(hw); > >> + unsigned long flags; > >> + u32 div; > >> + s32 inc1, inc2; > >> + bool was_enabled; > >> + > >> + div = 2 * parent_rate / rate; > >> + inc1 = 2 * parent_rate - div * rate; > >> + inc2 = -((s32) (rate - inc1)); > > > > Is the cast necessary? > > Answering that prompted me to get back to reading some C specification and now > I am confident in moving away from signed types here and in nco_recalc_rate > altogether. Great! Operating with only unsigned types makes this easier to understand. > >> + struct nco_tables *tbl; > >> + unsigned int nchannels; > >> + int ret, i; > >> + > >> + regs = devm_platform_get_and_ioremap_resource(pdev, 0, ®s_res); > >> + if (IS_ERR(regs)) > >> + return PTR_ERR(regs); > >> + > >> + if (resource_size(regs_res) < NCO_CHANNEL_REGSIZE) > >> + return -EINVAL; > >> + nchannels = (resource_size(regs_res) - NCO_CHANNEL_REGSIZE) > >> + / NCO_CHANNEL_STRIDE + 1; > > > > Is this some sort of DIV_ROUND_UP()? > > Almost. I will shop around for a macro replacement. Alright. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 3/3] MAINTAINERS: Add clk-apple-nco under ARM/APPLE MACHINE 2022-01-18 19:20 [PATCH v2 0/3] Support for Apple SoCs' NCO blocks Martin Povišer 2022-01-18 19:21 ` [PATCH v2 1/3] dt-bindings: clock: Add Apple NCO Martin Povišer 2022-01-18 19:21 ` [PATCH v2 2/3] clk: clk-apple-nco: Add driver for " Martin Povišer @ 2022-01-18 19:21 ` Martin Povišer 2 siblings, 0 replies; 10+ messages in thread From: Martin Povišer @ 2022-01-18 19:21 UTC (permalink / raw) To: mturquette, sboyd, robh+dt Cc: marcan, sven, alyssa, linux-clk, devicetree, linux-kernel, kettenis, Martin Povišer Add clk-apple-nco driver and corresponding binding schema to MAINTAINERS. Signed-off-by: Martin Povišer <povik+lin@protonmail.com> --- MAINTAINERS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 4e828542b089..9b6585717627 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1765,6 +1765,7 @@ C: irc://irc.oftc.net/asahi-dev T: git https://github.com/AsahiLinux/linux.git F: Documentation/devicetree/bindings/arm/apple.yaml F: Documentation/devicetree/bindings/arm/apple/* +F: Documentation/devicetree/bindings/clock/apple,nco.yaml F: Documentation/devicetree/bindings/i2c/apple,i2c.yaml F: Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml F: Documentation/devicetree/bindings/mailbox/apple,mailbox.yaml @@ -1773,6 +1774,7 @@ F: Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml F: Documentation/devicetree/bindings/power/apple* F: Documentation/devicetree/bindings/watchdog/apple,wdt.yaml F: arch/arm64/boot/dts/apple/ +F: drivers/clk/clk-apple-nco.c F: drivers/i2c/busses/i2c-pasemi-core.c F: drivers/i2c/busses/i2c-pasemi-platform.c F: drivers/irqchip/irq-apple-aic.c -- 2.33.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-01-20 20:59 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-01-18 19:20 [PATCH v2 0/3] Support for Apple SoCs' NCO blocks Martin Povišer 2022-01-18 19:21 ` [PATCH v2 1/3] dt-bindings: clock: Add Apple NCO Martin Povišer 2022-01-18 22:00 ` Mark Kettenis 2022-01-19 2:55 ` Rob Herring 2022-01-19 14:01 ` Rob Herring 2022-01-18 19:21 ` [PATCH v2 2/3] clk: clk-apple-nco: Add driver for " Martin Povišer 2022-01-20 5:38 ` Stephen Boyd 2022-01-20 12:11 ` Martin Povišer 2022-01-20 20:59 ` Stephen Boyd 2022-01-18 19:21 ` [PATCH v2 3/3] MAINTAINERS: Add clk-apple-nco under ARM/APPLE MACHINE Martin Povišer
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).