* [PATCH 0/2] add 74aup1g157gw 2-input multiplexer as clock driver @ 2018-10-10 2:15 Kuninori Morimoto 2018-10-10 2:16 ` [PATCH 1/2] dt-bindings: clock: add description of 74aup1g157gw Kuninori Morimoto 2018-10-10 2:16 ` [PATCH 2/2] clk: add 74aup1g157gw 2-input multiplexer as clock driver Kuninori Morimoto 0 siblings, 2 replies; 9+ messages in thread From: Kuninori Morimoto @ 2018-10-10 2:15 UTC (permalink / raw) To: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland Cc: linux-clk, devicetree, linux-kernel Hi Michael, Stephen, Rob, Mark These adds 74aup1g157gw 2-input multiplexer as clock driver. Kuninori Morimoto (2): dt-bindings: clock: add description of 74aup1g157gw clk: add 74aup1g157gw 2-input multiplexer as clock driver Documentation/devicetree/bindings/clock/nxp,74aup1g157gw.txt | 19 ++++++ drivers/clk/Kconfig | 1 + drivers/clk/Makefile | 2 +- drivers/clk/nxp/Kconfig | 4 ++ drivers/clk/nxp/Makefile | 1 + drivers/clk/nxp/clk-74aup1g157gw.c | 213 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 239 insertions(+), 1 deletion(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] dt-bindings: clock: add description of 74aup1g157gw 2018-10-10 2:15 [PATCH 0/2] add 74aup1g157gw 2-input multiplexer as clock driver Kuninori Morimoto @ 2018-10-10 2:16 ` Kuninori Morimoto 2018-10-17 16:05 ` Rob Herring 2018-10-10 2:16 ` [PATCH 2/2] clk: add 74aup1g157gw 2-input multiplexer as clock driver Kuninori Morimoto 1 sibling, 1 reply; 9+ messages in thread From: Kuninori Morimoto @ 2018-10-10 2:16 UTC (permalink / raw) To: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland Cc: linux-clk, devicetree, linux-kernel From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> 74aup1g157gw needs i0 and i1 pin as input, select and output it by sel gpio pin. This patch adds description for 74aup1g157gw as clock multiplexer. "nxp,74aup1g157gw-clk" will select most closest input as output, "nxp,74aup1g157gw-audio-clk" will select 48kHz/44.1kHz categorized input as output. Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> --- .../devicetree/bindings/clock/nxp,74aup1g157gw.txt | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/nxp,74aup1g157gw.txt diff --git a/Documentation/devicetree/bindings/clock/nxp,74aup1g157gw.txt b/Documentation/devicetree/bindings/clock/nxp,74aup1g157gw.txt new file mode 100644 index 0000000..d07bc04 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/nxp,74aup1g157gw.txt @@ -0,0 +1,19 @@ +NXP 74AUP1G157GW Low-power 2-input multiplexer for clock + +Required properties: +- compatible: "nxp,74aup1g157gw-clk" (select closest input as output) + "nxp,74aup1g157gw-audio-clk (select 48kHz / 44.1kHz as output) +- clocks: select input clock as i0, i1 +- clock-names: "i0" and "i1" +- sel-gpios: selector pin + +Examples: + + clk { + #clock-cells = <0>; + compatible = "nxp,74aup1g157gw-audio-clk"; + clocks = <&clk8snd>, <&clksnd>; + clock-names = "i0", "i1"; + sel-gpios = <&gpio_exp_75 13 GPIO_ACTIVE_HIGH>; + }; + -- 2.7.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] dt-bindings: clock: add description of 74aup1g157gw 2018-10-10 2:16 ` [PATCH 1/2] dt-bindings: clock: add description of 74aup1g157gw Kuninori Morimoto @ 2018-10-17 16:05 ` Rob Herring 2018-10-18 0:15 ` Kuninori Morimoto 0 siblings, 1 reply; 9+ messages in thread From: Rob Herring @ 2018-10-17 16:05 UTC (permalink / raw) To: Kuninori Morimoto Cc: Michael Turquette, Stephen Boyd, Mark Rutland, linux-clk, devicetree, linux-kernel On Wed, Oct 10, 2018 at 02:16:35AM +0000, Kuninori Morimoto wrote: > > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > 74aup1g157gw needs i0 and i1 pin as input, select and output it by > sel gpio pin. This patch adds description for 74aup1g157gw as clock > multiplexer. > "nxp,74aup1g157gw-clk" will select most closest input as output, > "nxp,74aup1g157gw-audio-clk" will select 48kHz/44.1kHz categorized > input as output. > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > --- > .../devicetree/bindings/clock/nxp,74aup1g157gw.txt | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > create mode 100644 Documentation/devicetree/bindings/clock/nxp,74aup1g157gw.txt > > diff --git a/Documentation/devicetree/bindings/clock/nxp,74aup1g157gw.txt b/Documentation/devicetree/bindings/clock/nxp,74aup1g157gw.txt > new file mode 100644 > index 0000000..d07bc04 > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/nxp,74aup1g157gw.txt > @@ -0,0 +1,19 @@ > +NXP 74AUP1G157GW Low-power 2-input multiplexer for clock > + > +Required properties: > +- compatible: "nxp,74aup1g157gw-clk" (select closest input as output) > + "nxp,74aup1g157gw-audio-clk (select 48kHz / 44.1kHz as output) I don't understand why you need the second one. What's wrong with the existing gpio-mux-clock binding? > +- clocks: select input clock as i0, i1 > +- clock-names: "i0" and "i1" > +- sel-gpios: selector pin > + > +Examples: > + > + clk { > + #clock-cells = <0>; > + compatible = "nxp,74aup1g157gw-audio-clk"; > + clocks = <&clk8snd>, <&clksnd>; > + clock-names = "i0", "i1"; > + sel-gpios = <&gpio_exp_75 13 GPIO_ACTIVE_HIGH>; > + }; > + > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] dt-bindings: clock: add description of 74aup1g157gw 2018-10-17 16:05 ` Rob Herring @ 2018-10-18 0:15 ` Kuninori Morimoto 0 siblings, 0 replies; 9+ messages in thread From: Kuninori Morimoto @ 2018-10-18 0:15 UTC (permalink / raw) To: Rob Herring Cc: Michael Turquette, Stephen Boyd, Mark Rutland, linux-clk, devicetree, linux-kernel Hi Rob Thank you for your feedback > > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > > > 74aup1g157gw needs i0 and i1 pin as input, select and output it by > > sel gpio pin. This patch adds description for 74aup1g157gw as clock > > multiplexer. > > "nxp,74aup1g157gw-clk" will select most closest input as output, > > "nxp,74aup1g157gw-audio-clk" will select 48kHz/44.1kHz categorized > > input as output. > > > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > --- > > .../devicetree/bindings/clock/nxp,74aup1g157gw.txt | 19 +++++++++++++++++++ > > 1 file changed, 19 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/clock/nxp,74aup1g157gw.txt > > > > diff --git a/Documentation/devicetree/bindings/clock/nxp,74aup1g157gw.txt b/Documentation/devicetree/bindings/clock/nxp,74aup1g157gw.txt > > new file mode 100644 > > index 0000000..d07bc04 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/clock/nxp,74aup1g157gw.txt > > @@ -0,0 +1,19 @@ > > +NXP 74AUP1G157GW Low-power 2-input multiplexer for clock > > + > > +Required properties: > > +- compatible: "nxp,74aup1g157gw-clk" (select closest input as output) > > + "nxp,74aup1g157gw-audio-clk (select 48kHz / 44.1kHz as output) > > I don't understand why you need the second one. > > > What's wrong with the existing gpio-mux-clock binding? Thanks. I already noticed that gpio-mux-clock is very OK to me (and I think I posted it to ML). Please drop this patch. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] clk: add 74aup1g157gw 2-input multiplexer as clock driver 2018-10-10 2:15 [PATCH 0/2] add 74aup1g157gw 2-input multiplexer as clock driver Kuninori Morimoto 2018-10-10 2:16 ` [PATCH 1/2] dt-bindings: clock: add description of 74aup1g157gw Kuninori Morimoto @ 2018-10-10 2:16 ` Kuninori Morimoto 2018-10-11 6:39 ` Stephen Boyd 1 sibling, 1 reply; 9+ messages in thread From: Kuninori Morimoto @ 2018-10-10 2:16 UTC (permalink / raw) To: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland Cc: linux-clk, devicetree, linux-kernel From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> 74aup1g157gw needs i0 and i1 pin as input, select and output it by sel gpio pin. This driver adds new 74aup1g157gw as clock multiplexer. "nxp,74aup1g157gw-clk" will select most closest input as output, "nxp,74aup1g157gw-audio-clk" will select 48kHz/44.1kHz categorized input as output. Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> --- drivers/clk/Kconfig | 1 + drivers/clk/Makefile | 2 +- drivers/clk/nxp/Kconfig | 4 + drivers/clk/nxp/Makefile | 1 + drivers/clk/nxp/clk-74aup1g157gw.c | 213 +++++++++++++++++++++++++++++++++++++ 5 files changed, 220 insertions(+), 1 deletion(-) create mode 100644 drivers/clk/nxp/Kconfig create mode 100644 drivers/clk/nxp/clk-74aup1g157gw.c diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig index 292056b..9cfeb0e 100644 --- a/drivers/clk/Kconfig +++ b/drivers/clk/Kconfig @@ -299,5 +299,6 @@ source "drivers/clk/sunxi-ng/Kconfig" source "drivers/clk/tegra/Kconfig" source "drivers/clk/ti/Kconfig" source "drivers/clk/uniphier/Kconfig" +source "drivers/clk/nxp/Kconfig" endmenu diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile index ed344eb..fcfd42a 100644 --- a/drivers/clk/Makefile +++ b/drivers/clk/Makefile @@ -84,7 +84,7 @@ obj-$(CONFIG_ARCH_MMP) += mmp/ endif obj-y += mvebu/ obj-$(CONFIG_ARCH_MXS) += mxs/ -obj-$(CONFIG_COMMON_CLK_NXP) += nxp/ +obj-y += nxp/ obj-$(CONFIG_MACH_PISTACHIO) += pistachio/ obj-$(CONFIG_COMMON_CLK_PXA) += pxa/ obj-$(CONFIG_COMMON_CLK_QCOM) += qcom/ diff --git a/drivers/clk/nxp/Kconfig b/drivers/clk/nxp/Kconfig new file mode 100644 index 0000000..44c8fed --- /dev/null +++ b/drivers/clk/nxp/Kconfig @@ -0,0 +1,4 @@ +config CLK_74AUP1G157GW + tristate "NXP 74AUP1G157GW Low-power 2-input multiplexer support as clock" + help + This enables NXP 74AUP1G157GW 2-input multiplexer as clock driver diff --git a/drivers/clk/nxp/Makefile b/drivers/clk/nxp/Makefile index d456ee6..2965bdd 100644 --- a/drivers/clk/nxp/Makefile +++ b/drivers/clk/nxp/Makefile @@ -2,3 +2,4 @@ obj-$(CONFIG_ARCH_LPC18XX) += clk-lpc18xx-cgu.o obj-$(CONFIG_ARCH_LPC18XX) += clk-lpc18xx-ccu.o obj-$(CONFIG_ARCH_LPC18XX) += clk-lpc18xx-creg.o obj-$(CONFIG_ARCH_LPC32XX) += clk-lpc32xx.o +obj-$(CONFIG_CLK_74AUP1G157GW) += clk-74aup1g157gw.o diff --git a/drivers/clk/nxp/clk-74aup1g157gw.c b/drivers/clk/nxp/clk-74aup1g157gw.c new file mode 100644 index 0000000..78199bb --- /dev/null +++ b/drivers/clk/nxp/clk-74aup1g157gw.c @@ -0,0 +1,213 @@ +// SPDX-License-Identifier: GPL-2.0 +// +// Copyright (C) 2018 Renesas Solutions Corp. +// Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> +// +#include <linux/module.h> +#include <linux/clk.h> +#include <linux/clk-provider.h> +#include <linux/slab.h> +#include <linux/err.h> +#include <linux/gpio/consumer.h> +#include <linux/of.h> +#include <linux/of_device.h> +#include <linux/platform_device.h> + +#define CLK_I_NUM 2 +struct clk_priv { + struct clk_hw hw; + struct device *dev; + struct clk *i[CLK_I_NUM]; + struct gpio_desc *sel; + long (*round_rate)(struct clk_hw *hw, unsigned long rate, + unsigned long *parent_rate); +}; +#define hw_to_priv(_hw) container_of(_hw, struct clk_priv, hw) +#define priv_to_dev(priv) priv->dev +#define for_each_iclk(i) \ + for ((i) = 0; (i) < CLK_I_NUM; (i)++) + +static int clk74_set_rate(struct clk_hw *hw, unsigned long rate, + unsigned long parent_rate) +{ + struct clk_priv *priv = hw_to_priv(hw); + struct device *dev = priv_to_dev(priv); + int i; + + for_each_iclk(i) { + if (rate == clk_get_rate(priv->i[i])) { + dev_dbg(dev, "set rate %lu as i%d\n", rate, i); + gpiod_set_value_cansleep(priv->sel, i); + return 0; + } + } + + dev_err(dev, "unsupported rate %lu\n", rate); + return -EIO; +} + +static long clk74_round_rate_close(struct clk_hw *hw, unsigned long rate, + unsigned long *parent_rate) +{ + struct clk_priv *priv = hw_to_priv(hw); + struct device *dev = priv_to_dev(priv); + unsigned long min = ~0; + unsigned long ret = 0; + int i; + + /* + * select closest rate + */ + for_each_iclk(i) { + unsigned long irate = clk_get_rate(priv->i[i]); + unsigned long diff = abs(rate - irate); + + if (min > diff) { + min = diff; + ret = irate; + } + } + + dev_dbg(dev, "(close)round rate %lu\n", ret); + + return ret; +} + +static long clk74_round_rate_audio(struct clk_hw *hw, unsigned long rate, + unsigned long *parent_rate) +{ + struct clk_priv *priv = hw_to_priv(hw); + struct device *dev = priv_to_dev(priv); + unsigned long ret = 0; + int is_8k = 0; + int i; + + /* + * select 48kHz or 44.1kHz category rate + */ + if (!(rate % 8000)) + is_8k = 1; + + for_each_iclk(i) { + unsigned long irate = clk_get_rate(priv->i[i]); + + if (( is_8k && !(irate % 8000)) || + (!is_8k && (irate % 8000))) { + ret = irate; + } + } + + dev_dbg(dev, "(audio)round rate %lu\n", ret); + + return ret; +} + +static long clk74_round_rate(struct clk_hw *hw, unsigned long rate, + unsigned long *parent_rate) +{ + struct clk_priv *priv = hw_to_priv(hw); + + return priv->round_rate(hw, rate, parent_rate); +} + +static unsigned long clk74_recalc_rate(struct clk_hw *hw, + unsigned long parent_rate) +{ + struct clk_priv *priv = hw_to_priv(hw); + struct device *dev = priv_to_dev(priv); + unsigned long rate; + int i = gpiod_get_raw_value_cansleep(priv->sel); + + rate = clk_get_rate(priv->i[i]); + + dev_dbg(dev, "recalc rate %lu as i%d\n", rate, i); + + return rate; +} + +static u8 clk74_get_parent(struct clk_hw *hw) +{ + struct clk_priv *priv = hw_to_priv(hw); + + return gpiod_get_raw_value_cansleep(priv->sel); +} + +static const struct clk_ops clk74_ops = { + .set_rate = clk74_set_rate, + .round_rate = clk74_round_rate, + .recalc_rate = clk74_recalc_rate, + .get_parent = clk74_get_parent, +}; + +static const char * const clk74_in_name[CLK_I_NUM] = { + "i0", + "i1", +}; + +static int clk74_probe(struct platform_device *pdev) +{ + struct clk *clk; + struct clk_init_data init; + struct clk_priv *priv; + struct device *dev = &pdev->dev; + const char *parent_names[CLK_I_NUM]; + int i, ret; + + priv = kzalloc(sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENODEV; + + for_each_iclk(i) { + clk = devm_clk_get(dev, clk74_in_name[i]); + if (IS_ERR(clk)) + return -EPROBE_DEFER; + priv->i[i] = clk; + parent_names[i] = __clk_get_name(clk); + } + + memset(&init, 0, sizeof(init)); + init.name = "74aup1g157gw"; + init.ops = &clk74_ops; + init.parent_names = parent_names; + init.num_parents = CLK_I_NUM; + + priv->hw.init = &init; + priv->dev = dev; + priv->round_rate = of_device_get_match_data(dev); + priv->sel = devm_gpiod_get(dev, "sel", 0); + if (IS_ERR(priv->sel)) + return -EPROBE_DEFER; + + gpiod_direction_output(priv->sel, 0); + + ret = devm_clk_hw_register(dev, &priv->hw); + if (ret < 0) + return ret; + + ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get, &priv->hw); + if (ret < 0) + return ret; + + platform_set_drvdata(pdev, priv); + + dev_info(dev, "probed\n"); + + return 0; +} + +#define OF_ID(name, func) { .compatible = name, .data = func } +static const struct of_device_id clk74_of_match[] = { + OF_ID("nxp,74aup1g157gw-clk", clk74_round_rate_close), + OF_ID("nxp,74aup1g157gw-audio-clk", clk74_round_rate_audio), + { } +}; +MODULE_DEVICE_TABLE(of, clk74_of_match); + +static struct platform_driver clk74_driver = { + .driver = { + .name = "74aup1g157gw", + .of_match_table = clk74_of_match, + }, + .probe = clk74_probe, +}; +builtin_platform_driver(clk74_driver); -- 2.7.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] clk: add 74aup1g157gw 2-input multiplexer as clock driver 2018-10-10 2:16 ` [PATCH 2/2] clk: add 74aup1g157gw 2-input multiplexer as clock driver Kuninori Morimoto @ 2018-10-11 6:39 ` Stephen Boyd 2018-10-12 0:37 ` Kuninori Morimoto 0 siblings, 1 reply; 9+ messages in thread From: Stephen Boyd @ 2018-10-11 6:39 UTC (permalink / raw) To: Kuninori Morimoto, Mark Rutland, Michael Turquette, Rob Herring Cc: linux-clk, devicetree, linux-kernel Quoting Kuninori Morimoto (2018-10-09 19:16:48) > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig > index 292056b..9cfeb0e 100644 > --- a/drivers/clk/Kconfig > +++ b/drivers/clk/Kconfig > @@ -299,5 +299,6 @@ source "drivers/clk/sunxi-ng/Kconfig" > source "drivers/clk/tegra/Kconfig" > source "drivers/clk/ti/Kconfig" > source "drivers/clk/uniphier/Kconfig" > +source "drivers/clk/nxp/Kconfig" Please sort this alphabetically. > > endmenu > diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile > index ed344eb..fcfd42a 100644 > --- a/drivers/clk/Makefile > +++ b/drivers/clk/Makefile > @@ -84,7 +84,7 @@ obj-$(CONFIG_ARCH_MMP) += mmp/ > endif > obj-y += mvebu/ > obj-$(CONFIG_ARCH_MXS) += mxs/ > -obj-$(CONFIG_COMMON_CLK_NXP) += nxp/ > +obj-y += nxp/ There's the CONFIG_COMMON_CLK_NXP config symbol that could go into the drivers/clk/nxp/Kconfig file then and also wrap the Makefile contents in the nxp subdirectory too? We don't want to lose that config option and start enabling this driver all the time. > obj-$(CONFIG_MACH_PISTACHIO) += pistachio/ > obj-$(CONFIG_COMMON_CLK_PXA) += pxa/ > obj-$(CONFIG_COMMON_CLK_QCOM) += qcom/ > diff --git a/drivers/clk/nxp/clk-74aup1g157gw.c b/drivers/clk/nxp/clk-74aup1g157gw.c > new file mode 100644 > index 0000000..78199bb > --- /dev/null > +++ b/drivers/clk/nxp/clk-74aup1g157gw.c > @@ -0,0 +1,213 @@ > +// SPDX-License-Identifier: GPL-2.0 > +// > +// Copyright (C) 2018 Renesas Solutions Corp. > +// Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > +// > +#include <linux/module.h> > +#include <linux/clk.h> > +#include <linux/clk-provider.h> > +#include <linux/slab.h> > +#include <linux/err.h> > +#include <linux/gpio/consumer.h> > +#include <linux/of.h> > +#include <linux/of_device.h> > +#include <linux/platform_device.h> > + > +#define CLK_I_NUM 2 > +struct clk_priv { > + struct clk_hw hw; > + struct device *dev; > + struct clk *i[CLK_I_NUM]; > + struct gpio_desc *sel; > + long (*round_rate)(struct clk_hw *hw, unsigned long rate, > + unsigned long *parent_rate); > +}; > +#define hw_to_priv(_hw) container_of(_hw, struct clk_priv, hw) > +#define priv_to_dev(priv) priv->dev Nitpick: This macro is just obfuscating, there isn't any container_of() usage going on so please remove it. > +#define for_each_iclk(i) \ > + for ((i) = 0; (i) < CLK_I_NUM; (i)++) Nitpick: This macros isn't making things any shorter, so please remove it. > + > +static int clk74_set_rate(struct clk_hw *hw, unsigned long rate, > + unsigned long parent_rate) > +{ > + struct clk_priv *priv = hw_to_priv(hw); > + struct device *dev = priv_to_dev(priv); > + int i; > + > + for_each_iclk(i) { > + if (rate == clk_get_rate(priv->i[i])) { > + dev_dbg(dev, "set rate %lu as i%d\n", rate, i); > + gpiod_set_value_cansleep(priv->sel, i); > + return 0; > + } > + } > + > + dev_err(dev, "unsupported rate %lu\n", rate); > + return -EIO; > +} > + > +static long clk74_round_rate_close(struct clk_hw *hw, unsigned long rate, > + unsigned long *parent_rate) > +{ > + struct clk_priv *priv = hw_to_priv(hw); > + struct device *dev = priv_to_dev(priv); > + unsigned long min = ~0; > + unsigned long ret = 0; > + int i; > + > + /* > + * select closest rate > + */ > + for_each_iclk(i) { > + unsigned long irate = clk_get_rate(priv->i[i]); > + unsigned long diff = abs(rate - irate); > + > + if (min > diff) { > + min = diff; > + ret = irate; > + } > + } > + > + dev_dbg(dev, "(close)round rate %lu\n", ret); > + > + return ret; > +} > + > +static long clk74_round_rate_audio(struct clk_hw *hw, unsigned long rate, > + unsigned long *parent_rate) > +{ > + struct clk_priv *priv = hw_to_priv(hw); > + struct device *dev = priv_to_dev(priv); > + unsigned long ret = 0; > + int is_8k = 0; > + int i; > + > + /* > + * select 48kHz or 44.1kHz category rate > + */ > + if (!(rate % 8000)) > + is_8k = 1; > + > + for_each_iclk(i) { > + unsigned long irate = clk_get_rate(priv->i[i]); > + > + if (( is_8k && !(irate % 8000)) || > + (!is_8k && (irate % 8000))) { > + ret = irate; > + } > + } > + > + dev_dbg(dev, "(audio)round rate %lu\n", ret); > + > + return ret; > +} > + > +static long clk74_round_rate(struct clk_hw *hw, unsigned long rate, > + unsigned long *parent_rate) > +{ > + struct clk_priv *priv = hw_to_priv(hw); > + > + return priv->round_rate(hw, rate, parent_rate); > +} > + > +static unsigned long clk74_recalc_rate(struct clk_hw *hw, > + unsigned long parent_rate) > +{ > + struct clk_priv *priv = hw_to_priv(hw); > + struct device *dev = priv_to_dev(priv); > + unsigned long rate; > + int i = gpiod_get_raw_value_cansleep(priv->sel); > + > + rate = clk_get_rate(priv->i[i]); > + > + dev_dbg(dev, "recalc rate %lu as i%d\n", rate, i); > + > + return rate; > +} > + > +static u8 clk74_get_parent(struct clk_hw *hw) > +{ > + struct clk_priv *priv = hw_to_priv(hw); > + > + return gpiod_get_raw_value_cansleep(priv->sel); > +} > + > +static const struct clk_ops clk74_ops = { > + .set_rate = clk74_set_rate, > + .round_rate = clk74_round_rate, > + .recalc_rate = clk74_recalc_rate, > + .get_parent = clk74_get_parent, > +}; Can this all be handled by the 'gpio-mux-clock' compatible/driver? I suppose it may need an update to add the rounding policy that you want via some sort of DT property, but otherwise it would be fine? > + > +static const char * const clk74_in_name[CLK_I_NUM] = { > + "i0", > + "i1", > +}; > + > +static int clk74_probe(struct platform_device *pdev) > +{ > + struct clk *clk; > + struct clk_init_data init; > + struct clk_priv *priv; > + struct device *dev = &pdev->dev; > + const char *parent_names[CLK_I_NUM]; > + int i, ret; > + > + priv = kzalloc(sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENODEV; You mean -ENOMEM? > + > + for_each_iclk(i) { > + clk = devm_clk_get(dev, clk74_in_name[i]); > + if (IS_ERR(clk)) > + return -EPROBE_DEFER; Please return actual error code instead of overriding and returning probe defer. > + priv->i[i] = clk; > + parent_names[i] = __clk_get_name(clk); > + } > + > + memset(&init, 0, sizeof(init)); > + init.name = "74aup1g157gw"; > + init.ops = &clk74_ops; > + init.parent_names = parent_names; > + init.num_parents = CLK_I_NUM; > + > + priv->hw.init = &init; > + priv->dev = dev; > + priv->round_rate = of_device_get_match_data(dev); > + priv->sel = devm_gpiod_get(dev, "sel", 0); > + if (IS_ERR(priv->sel)) > + return -EPROBE_DEFER; Please return the actual error. > + > + gpiod_direction_output(priv->sel, 0); > + > + ret = devm_clk_hw_register(dev, &priv->hw); > + if (ret < 0) > + return ret; > + > + ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get, &priv->hw); > + if (ret < 0) > + return ret; > + > + platform_set_drvdata(pdev, priv); > + > + dev_info(dev, "probed\n"); Please remove noise like this. > + > + return 0; > +} > + > +#define OF_ID(name, func) { .compatible = name, .data = func } > +static const struct of_device_id clk74_of_match[] = { > + OF_ID("nxp,74aup1g157gw-clk", clk74_round_rate_close), > + OF_ID("nxp,74aup1g157gw-audio-clk", clk74_round_rate_audio), Nitpick: I'd prefer to not have the macro. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] clk: add 74aup1g157gw 2-input multiplexer as clock driver 2018-10-11 6:39 ` Stephen Boyd @ 2018-10-12 0:37 ` Kuninori Morimoto 2018-10-12 16:06 ` Stephen Boyd 0 siblings, 1 reply; 9+ messages in thread From: Kuninori Morimoto @ 2018-10-12 0:37 UTC (permalink / raw) To: Stephen Boyd Cc: Mark Rutland, Michael Turquette, Rob Herring, linux-clk, devicetree, linux-kernel Hi Stephen Thank you for your feedback > > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig > > index 292056b..9cfeb0e 100644 > > --- a/drivers/clk/Kconfig > > +++ b/drivers/clk/Kconfig > > @@ -299,5 +299,6 @@ source "drivers/clk/sunxi-ng/Kconfig" > > source "drivers/clk/tegra/Kconfig" > > source "drivers/clk/ti/Kconfig" > > source "drivers/clk/uniphier/Kconfig" > > +source "drivers/clk/nxp/Kconfig" > > Please sort this alphabetically. Will do > > obj-$(CONFIG_ARCH_MXS) += mxs/ > > -obj-$(CONFIG_COMMON_CLK_NXP) += nxp/ > > +obj-y += nxp/ > > There's the CONFIG_COMMON_CLK_NXP config symbol that could go into the > drivers/clk/nxp/Kconfig file then and also wrap the Makefile contents in > the nxp subdirectory too? We don't want to lose that config option and > start enabling this driver all the time. OK, will do > Nitpick: This macro is just obfuscating, there isn't any container_of() > usage going on so please remove it. (snip) > Nitpick: This macros isn't making things any shorter, so please remove > it. will fix > > +static const struct clk_ops clk74_ops = { > > + .set_rate = clk74_set_rate, > > + .round_rate = clk74_round_rate, > > + .recalc_rate = clk74_recalc_rate, > > + .get_parent = clk74_get_parent, > > +}; > > Can this all be handled by the 'gpio-mux-clock' compatible/driver? I > suppose it may need an update to add the rounding policy that you want > via some sort of DT property, but otherwise it would be fine? Hmm.. not sure. If we can add new feature (= .round_rate ?) on gpio-mux-clock, I can consider it. > You mean -ENOMEM? (snip) > Please return actual error code instead of overriding and returning > probe defer. (snip) > Please return the actual error. (snip) > Please remove noise like this. (snip) > Nitpick: I'd prefer to not have the macro. will fix Best regards --- Kuninori Morimoto ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] clk: add 74aup1g157gw 2-input multiplexer as clock driver 2018-10-12 0:37 ` Kuninori Morimoto @ 2018-10-12 16:06 ` Stephen Boyd 2018-10-14 23:53 ` Kuninori Morimoto 0 siblings, 1 reply; 9+ messages in thread From: Stephen Boyd @ 2018-10-12 16:06 UTC (permalink / raw) To: Kuninori Morimoto Cc: Mark Rutland, Michael Turquette, Rob Herring, linux-clk, devicetree, linux-kernel Quoting Kuninori Morimoto (2018-10-11 17:37:30) > > > + .recalc_rate = clk74_recalc_rate, > > > + .get_parent = clk74_get_parent, > > > +}; > > > > Can this all be handled by the 'gpio-mux-clock' compatible/driver? I > > suppose it may need an update to add the rounding policy that you want > > via some sort of DT property, but otherwise it would be fine? > > Hmm.. not sure. > If we can add new feature (= .round_rate ?) on gpio-mux-clock, > I can consider it. Yes that would be the idea. Extend gpio-mux-clock driver to have what you want with rounding. I'm not really sure why there is a rounding policy needed though. Is it a static configuration at boot, or does the code using this gpio clk need to search the parent rate space somehow and mux it over? ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] clk: add 74aup1g157gw 2-input multiplexer as clock driver 2018-10-12 16:06 ` Stephen Boyd @ 2018-10-14 23:53 ` Kuninori Morimoto 0 siblings, 0 replies; 9+ messages in thread From: Kuninori Morimoto @ 2018-10-14 23:53 UTC (permalink / raw) To: Stephen Boyd Cc: Mark Rutland, Michael Turquette, Rob Herring, linux-clk, devicetree, linux-kernel Hi Stephen > > > > + .recalc_rate = clk74_recalc_rate, > > > > + .get_parent = clk74_get_parent, > > > > +}; > > > > > > Can this all be handled by the 'gpio-mux-clock' compatible/driver? I > > > suppose it may need an update to add the rounding policy that you want > > > via some sort of DT property, but otherwise it would be fine? > > > > Hmm.. not sure. > > If we can add new feature (= .round_rate ?) on gpio-mux-clock, > > I can consider it. > > Yes that would be the idea. Extend gpio-mux-clock driver to have what > you want with rounding. I'm not really sure why there is a rounding > policy needed though. Is it a static configuration at boot, or does the > code using this gpio clk need to search the parent rate space somehow > and mux it over? Thank you for pointing gpio-mux-clock. I tried to use it, and it works well instead of new driver. Actually, it is not 100%, but it should be solved sound side, not clock side, I think. But anyway, this driver is no longer needed, please drop it. Thank you very much ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-10-18 0:15 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-10-10 2:15 [PATCH 0/2] add 74aup1g157gw 2-input multiplexer as clock driver Kuninori Morimoto 2018-10-10 2:16 ` [PATCH 1/2] dt-bindings: clock: add description of 74aup1g157gw Kuninori Morimoto 2018-10-17 16:05 ` Rob Herring 2018-10-18 0:15 ` Kuninori Morimoto 2018-10-10 2:16 ` [PATCH 2/2] clk: add 74aup1g157gw 2-input multiplexer as clock driver Kuninori Morimoto 2018-10-11 6:39 ` Stephen Boyd 2018-10-12 0:37 ` Kuninori Morimoto 2018-10-12 16:06 ` Stephen Boyd 2018-10-14 23:53 ` Kuninori Morimoto
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).