* [PATCH v3 2/3] ARM: dts: imx6: Support Savageboard dual
From: Milo Kim @ 2016-12-09 1:04 UTC (permalink / raw)
To: Shawn Guo, Sascha Hauer
Cc: Fabio Estevam, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Milo Kim
In-Reply-To: <20161209010436.7994-1-woogyom.kim-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Common savageboard DT file is used for board support.
Specify this dtb file for i.MX6Q build.
Signed-off-by: Milo Kim <woogyom.kim-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
arch/arm/boot/dts/Makefile | 1 +
arch/arm/boot/dts/imx6dl-savageboard.dts | 50 ++++++++++++++++++++++++++++++++
2 files changed, 51 insertions(+)
create mode 100644 arch/arm/boot/dts/imx6dl-savageboard.dts
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index c558ba7..64660c7 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -348,6 +348,7 @@ dtb-$(CONFIG_SOC_IMX6Q) += \
imx6dl-sabreauto.dtb \
imx6dl-sabrelite.dtb \
imx6dl-sabresd.dtb \
+ imx6dl-savageboard.dtb \
imx6dl-ts4900.dtb \
imx6dl-tx6dl-comtft.dtb \
imx6dl-tx6s-8034.dtb \
diff --git a/arch/arm/boot/dts/imx6dl-savageboard.dts b/arch/arm/boot/dts/imx6dl-savageboard.dts
new file mode 100644
index 0000000..2cac30d
--- /dev/null
+++ b/arch/arm/boot/dts/imx6dl-savageboard.dts
@@ -0,0 +1,50 @@
+/*
+ * Copyright (C) 2016 Milo Kim <woogyom.kim-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ * a) This file is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This file is distributed in the hope that it will be useful
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * Or, alternatively
+ *
+ * b) Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use
+ * copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following
+ * conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED , WITHOUT WARRANTY OF ANY KIND
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY
+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/dts-v1/;
+
+#include "imx6dl.dtsi"
+#include "imx6qdl-savageboard.dtsi"
+
+/ {
+ model = "Poslab SavageBoard Dual";
+ compatible = "poslab,imx6dl-savageboard", "fsl,imx6dl";
+};
--
2.9.3
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v3 1/3] ARM: dts: imx6: Add Savageboard common file
From: Milo Kim @ 2016-12-09 1:04 UTC (permalink / raw)
To: Shawn Guo, Sascha Hauer
Cc: Fabio Estevam, linux-arm-kernel, devicetree, linux-kernel,
Milo Kim
In-Reply-To: <20161209010436.7994-1-woogyom.kim@gmail.com>
* Memory
memblock for DDR3 1GB
* Regulator
3.3V for panel and backlight.
* Display
Enable HDMI and LVDS panel. Savageboard supports AVIC TM097TDH02 panel
which is compatible with Hannstar HSD100PXN1, so reuse it.
* Clock
The commit d28be499c45e6 is applied to support LVDS and HDMI output
simultaneously.
* Pinmux
eMMC, ethernet, HDMI, I2C, power button, PWM, SD card and UART.
* Others
Enable ethernet, UART1 debug, USB host, USDHC3 for microSD card and
USDHC4 for built-in eMMC storage.
Signed-off-by: Milo Kim <woogyom.kim@gmail.com>
---
arch/arm/boot/dts/imx6qdl-savageboard.dtsi | 262 +++++++++++++++++++++++++++++
1 file changed, 262 insertions(+)
create mode 100644 arch/arm/boot/dts/imx6qdl-savageboard.dtsi
diff --git a/arch/arm/boot/dts/imx6qdl-savageboard.dtsi b/arch/arm/boot/dts/imx6qdl-savageboard.dtsi
new file mode 100644
index 0000000..a7a7e1d
--- /dev/null
+++ b/arch/arm/boot/dts/imx6qdl-savageboard.dtsi
@@ -0,0 +1,262 @@
+/*
+ * Copyright (C) 2016 Milo Kim <woogyom.kim@gmail.com>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ * a) This file is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This file is distributed in the hope that it will be useful
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * Or, alternatively
+ *
+ * b) Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use
+ * copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following
+ * conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED , WITHOUT WARRANTY OF ANY KIND
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY
+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
+
+/ {
+ chosen {
+ stdout-path = &uart1;
+ };
+
+ memory@10000000 {
+ device_type = "memory";
+ reg = <0x10000000 0x40000000>;
+ };
+
+ backlight: panel_bl {
+ compatible = "pwm-backlight";
+ brightness-levels = <0 4 8 16 32 64 128 255>;
+ default-brightness-level = <4>;
+ power-supply = <®_3p3v>;
+ pwms = <&pwm1 0 10000>;
+ };
+
+ gpio-keys {
+ compatible = "gpio-keys";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_gpio_keys>;
+
+ power {
+ gpios = <&gpio3 7 GPIO_ACTIVE_LOW>;
+ label = "Power Button";
+ linux,code = <KEY_POWER>;
+ wakeup-source;
+ };
+ };
+
+ panel {
+ compatible = "avic, tm097tdh02", "hannstar,hsd100pxn1";
+ backlight = <&backlight>;
+ power-supply = <®_3p3v>;
+
+ port {
+ panel_in: endpoint {
+ remote-endpoint = <&lvds0_out>;
+ };
+ };
+ };
+
+ reg_3p3v: regulator-3p3v {
+ compatible = "regulator-fixed";
+ regulator-name = "3P3V";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-always-on;
+ };
+};
+
+&clks {
+ assigned-clocks = <&clks IMX6QDL_CLK_LDB_DI0_SEL>,
+ <&clks IMX6QDL_CLK_LDB_DI1_SEL>;
+ assigned-clock-parents = <&clks IMX6QDL_CLK_PLL3_USB_OTG>,
+ <&clks IMX6QDL_CLK_PLL3_USB_OTG>;
+};
+
+&fec {
+ phy-mode = "rgmii";
+ phy-reset-gpios = <&gpio1 25 GPIO_ACTIVE_LOW>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_enet>;
+ status = "okay";
+};
+
+&hdmi {
+ ddc-i2c-bus = <&i2c2>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_hdmi_tx_cec>;
+ status = "okay";
+};
+
+&i2c2 {
+ clock-frequency = <100000>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_i2c2>;
+ status = "okay";
+};
+
+&ldb {
+ status = "okay";
+
+ lvds-channel@0 {
+ reg = <0>;
+ status = "okay";
+
+ port@4 {
+ reg = <4>;
+
+ lvds0_out: endpoint {
+ remote-endpoint = <&panel_in>;
+ };
+ };
+ };
+};
+
+&pwm1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_pwm1>;
+ status = "okay";
+};
+
+&uart1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_uart1>;
+ status = "okay";
+};
+
+&usbh1 {
+ status = "okay";
+};
+
+/* SD card */
+&usdhc3 {
+ bus-width = <4>;
+ cd-gpios = <&gpio2 0 GPIO_ACTIVE_LOW>;
+ no-1-8-v;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_sd>;
+ status = "okay";
+};
+
+/* eMMC */
+&usdhc4 {
+ bus-width = <8>;
+ keep-power-in-suspend;
+ no-1-8-v;
+ non-removable;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_emmc>;
+ status = "okay";
+};
+
+&iomuxc {
+ pinctrl_emmc: emmcgrp {
+ fsl,pins = <
+ MX6QDL_PAD_SD4_CMD__SD4_CMD 0x17059
+ MX6QDL_PAD_SD4_CLK__SD4_CLK 0x10059
+ MX6QDL_PAD_SD4_DAT0__SD4_DATA0 0x17059
+ MX6QDL_PAD_SD4_DAT1__SD4_DATA1 0x17059
+ MX6QDL_PAD_SD4_DAT2__SD4_DATA2 0x17059
+ MX6QDL_PAD_SD4_DAT3__SD4_DATA3 0x17059
+ MX6QDL_PAD_SD4_DAT4__SD4_DATA4 0x17059
+ MX6QDL_PAD_SD4_DAT5__SD4_DATA5 0x17059
+ MX6QDL_PAD_SD4_DAT6__SD4_DATA6 0x17059
+ MX6QDL_PAD_SD4_DAT7__SD4_DATA7 0x17059
+ >;
+ };
+
+ pinctrl_enet: enetgrp {
+ fsl,pins = <
+ MX6QDL_PAD_ENET_MDIO__ENET_MDIO 0x1b0b0
+ MX6QDL_PAD_ENET_MDC__ENET_MDC 0x1b0b0
+ MX6QDL_PAD_RGMII_TXC__RGMII_TXC 0x1b030
+ MX6QDL_PAD_RGMII_TD0__RGMII_TD0 0x1b030
+ MX6QDL_PAD_RGMII_TD1__RGMII_TD1 0x1b030
+ MX6QDL_PAD_RGMII_TD2__RGMII_TD2 0x1b030
+ MX6QDL_PAD_RGMII_TD3__RGMII_TD3 0x1b030
+ MX6QDL_PAD_RGMII_TX_CTL__RGMII_TX_CTL 0x1b030
+ MX6QDL_PAD_ENET_REF_CLK__ENET_TX_CLK 0x1b0b0
+ MX6QDL_PAD_RGMII_RXC__RGMII_RXC 0x1b030
+ MX6QDL_PAD_RGMII_RD0__RGMII_RD0 0x1b030
+ MX6QDL_PAD_RGMII_RD1__RGMII_RD1 0x1b030
+ MX6QDL_PAD_RGMII_RD2__RGMII_RD2 0x1b030
+ MX6QDL_PAD_RGMII_RD3__RGMII_RD3 0x1b030
+ MX6QDL_PAD_RGMII_RX_CTL__RGMII_RX_CTL 0x1b030
+ /* PHY reset */
+ MX6QDL_PAD_ENET_CRS_DV__GPIO1_IO25 0x1b0b0
+ >;
+ };
+
+ pinctrl_hdmi_tx_cec: hdmitxcecgrp {
+ fsl,pins = <
+ MX6QDL_PAD_KEY_ROW2__HDMI_TX_CEC_LINE 0x1f8b0
+ >;
+ };
+
+ pinctrl_i2c2: i2c2grp {
+ fsl,pins = <
+ MX6QDL_PAD_KEY_COL3__I2C2_SCL 0x4001b8b1
+ MX6QDL_PAD_KEY_ROW3__I2C2_SDA 0x4001b8b1
+ >;
+ };
+
+ pinctrl_gpio_keys: gpiokeysgrp {
+ fsl,pins = <
+ MX6QDL_PAD_EIM_DA7__GPIO3_IO07 0x1b0b1
+ >;
+ };
+
+ pinctrl_pwm1: pwm1grp {
+ fsl,pins = <
+ MX6QDL_PAD_SD1_DAT3__PWM1_OUT 0x1b0b1
+ >;
+ };
+
+ pinctrl_sd: sdgrp {
+ fsl,pins = <
+ MX6QDL_PAD_SD3_CMD__SD3_CMD 0x17059
+ MX6QDL_PAD_SD3_CLK__SD3_CLK 0x10059
+ MX6QDL_PAD_SD3_DAT0__SD3_DATA0 0x17059
+ MX6QDL_PAD_SD3_DAT1__SD3_DATA1 0x17059
+ MX6QDL_PAD_SD3_DAT2__SD3_DATA2 0x17059
+ MX6QDL_PAD_SD3_DAT3__SD3_DATA3 0x17059
+ /* CD pin */
+ MX6QDL_PAD_NANDF_D0__GPIO2_IO00 0x1b0b1
+ >;
+ };
+
+ pinctrl_uart1: uart1grp {
+ fsl,pins = <
+ MX6QDL_PAD_CSI0_DAT10__UART1_TX_DATA 0x1b0b1
+ MX6QDL_PAD_CSI0_DAT11__UART1_RX_DATA 0x1b0b1
+ >;
+ };
+};
--
2.9.3
^ permalink raw reply related
* [PATCH v3 0/3] ARM: dts: imx6: Support Poslab Savageboard dual & quad
From: Milo Kim @ 2016-12-09 1:04 UTC (permalink / raw)
To: Shawn Guo, Sascha Hauer
Cc: Fabio Estevam, linux-arm-kernel, devicetree, linux-kernel,
Milo Kim
Poslab Savageboard is i.MX6 SoC base, but BSP code from the vendor is
not mainline u-boot and kernel. Personal reason of using this board is
testing etnaviv user-space driver, so I re-write device tree files based on
mainline kernel for the first step.
This patchset includes common DT file, dual and quad board files.
Supported components are
- Display: HDMI and LVDS panel
- eMMC and SD card
- Ethernet
- Pinmux configuration
- SATA: only for Savageboard quad
- UART1 for debug console
- USB host
Missing features are
- Audio (WM8903)
- USB OTG
- PMIC WM8326: default settings are used so no issue to bring-up the system
- MIPI DSI, CSI
Patches are tested on the Savageboard quad but the dual version should work
because the only difference between dual and quad is SATA support.
More information in http://www.savageboard.org
v3:
Specify the dtbs for i.MX6 build.
v2:
Fix DT node for regulator, phy-reset-gpios and iomuxc node.
Milo Kim (3):
ARM: dts: imx6: Add Savageboard common file
ARM: dts: imx6: Support Savageboard dual
ARM: dts: imx6: Support Savageboard quad
arch/arm/boot/dts/Makefile | 2 +
arch/arm/boot/dts/imx6dl-savageboard.dts | 50 ++++++
arch/arm/boot/dts/imx6q-savageboard.dts | 54 ++++++
arch/arm/boot/dts/imx6qdl-savageboard.dtsi | 262 +++++++++++++++++++++++++++++
4 files changed, 368 insertions(+)
create mode 100644 arch/arm/boot/dts/imx6dl-savageboard.dts
create mode 100644 arch/arm/boot/dts/imx6q-savageboard.dts
create mode 100644 arch/arm/boot/dts/imx6qdl-savageboard.dtsi
--
2.9.3
^ permalink raw reply
* Re: [PATCH 3/3] ASoC: simple-card-utils: enable clocks/clock-names/clock-ranges
From: Kuninori Morimoto @ 2016-12-09 0:55 UTC (permalink / raw)
To: Stephen Boyd
Cc: Russell King - ARM Linux, Rob Herring, Linux-ALSA, Linux-DT,
Michael Turquette, Linux-Kernel, Mark Brown, linux-clk, Linux-ARM
In-Reply-To: <20161209002635.GD5423@codeaurora.org>
Hi Stephen
> > > I don't see any reason why we need this patch though. The binding
> > > works as is, so supporting different styles doesn't seem like a
> > > good idea to me. Let's just keep what we have? Even if a sub-node
> > > like cpu or codec gets more than one element in the clocks list
> > > property, we can make that work by passing a clock-name then
> > > based on some sort of other knowledge.
> >
> > OK, thanks. Let's skip this patch.
> > But I believe this idea/method itself is not wrong (?)
> >
> Right it's not wrong, just seems confusing to have two methods.
Thanks.
Very clear for me :)
^ permalink raw reply
* Re: [PATCH 2/6] net: ethernet: ti: cpts: add support for ext rftclk selection
From: Stephen Boyd @ 2016-12-09 0:47 UTC (permalink / raw)
To: Grygorii Strashko
Cc: Richard Cochran, Murali Karicheri, David S. Miller, netdev,
Mugunthan V N, Sekhar Nori, linux-kernel, linux-omap, Rob Herring,
devicetree, Wingman Kwok, linux-clk
In-Reply-To: <11994fbc-3713-6ef7-8a44-8a2442106dfc@ti.com>
On 12/06, Grygorii Strashko wrote:
> Subject: [PATCH] cpts refclk sel
>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
> arch/arm/boot/dts/keystone-k2e-netcp.dtsi | 10 +++++-
> drivers/net/ethernet/ti/cpts.c | 52 ++++++++++++++++++++++++++++++-
> 2 files changed, 60 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/boot/dts/keystone-k2e-netcp.dtsi b/arch/arm/boot/dts/keystone-k2e-netcp.dtsi
> index 919e655..b27aa22 100644
> --- a/arch/arm/boot/dts/keystone-k2e-netcp.dtsi
> +++ b/arch/arm/boot/dts/keystone-k2e-netcp.dtsi
> @@ -138,7 +138,7 @@ netcp: netcp@24000000 {
> /* NetCP address range */
> ranges = <0 0x24000000 0x1000000>;
>
> - clocks = <&clkpa>, <&clkcpgmac>, <&chipclk12>;
> + clocks = <&clkpa>, <&clkcpgmac>, <&cpts_mux>;
> clock-names = "pa_clk", "ethss_clk", "cpts";
> dma-coherent;
>
> @@ -162,6 +162,14 @@ netcp: netcp@24000000 {
> cpts-ext-ts-inputs = <6>;
> cpts-ts-comp-length;
>
> + cpts_mux: cpts_refclk_mux {
> + #clock-cells = <0>;
> + clocks = <&chipclk12>, <&chipclk13>;
> + cpts-mux-tbl = <0>, <1>;
> + assigned-clocks = <&cpts_mux>;
> + assigned-clock-parents = <&chipclk12>;
Is there a binding update? Why the subnode? Why not have it as
part of the netcp node? Does the cpts-mux-tbl property change?
> + };
> +
> interfaces {
> gbe0: interface-0 {
> slave-port = <0>;
> diff --git a/drivers/net/ethernet/ti/cpts.c b/drivers/net/ethernet/ti/cpts.c
> index 938de22..ef94316 100644
> --- a/drivers/net/ethernet/ti/cpts.c
> +++ b/drivers/net/ethernet/ti/cpts.c
> @@ -17,6 +17,7 @@
> * along with this program; if not, write to the Free Software
> * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> */
> +#include <linux/clk-provider.h>
> #include <linux/err.h>
> #include <linux/if.h>
> #include <linux/hrtimer.h>
> @@ -672,6 +673,7 @@ int cpts_register(struct cpts *cpts)
> cpts->phc_index = ptp_clock_index(cpts->clock);
>
> schedule_delayed_work(&cpts->overflow_work, cpts->ov_check_period);
> +
Maybe in another patch.
> return 0;
>
> err_ptp:
> @@ -741,6 +743,54 @@ static void cpts_calc_mult_shift(struct cpts *cpts)
> freq, cpts->cc_mult, cpts->cc.shift, (ns - NSEC_PER_SEC));
> }
>
> +static int cpts_of_mux_clk_setup(struct cpts *cpts, struct device_node *node)
> +{
> + unsigned int num_parents;
> + const char **parent_names;
> + struct device_node *refclk_np;
> + void __iomem *reg;
> + struct clk *clk;
> + u32 *mux_table;
> + int ret;
> +
> + refclk_np = of_get_child_by_name(node, "cpts_refclk_mux");
> + if (!refclk_np)
> + return -EINVAL;
> +
> + num_parents = of_clk_get_parent_count(refclk_np);
> + if (num_parents < 1) {
> + dev_err(cpts->dev, "mux-clock %s must have parents\n",
> + refclk_np->name);
> + return -EINVAL;
> + }
> + parent_names = devm_kzalloc(cpts->dev, (sizeof(char *) * num_parents),
> + GFP_KERNEL);
> + if (!parent_names)
> + return -ENOMEM;
> +
> + of_clk_parent_fill(refclk_np, parent_names, num_parents);
> +
> + mux_table = devm_kzalloc(cpts->dev, sizeof(*mux_table) * (32 + 1),
> + GFP_KERNEL);
> + if (!mux_table)
> + return -ENOMEM;
> +
> + ret = of_property_read_variable_u32_array(refclk_np, "cpts-mux-tbl",
> + mux_table, 1, 32);
> + if (ret < 0)
> + return ret;
> +
> + reg = &cpts->reg->rftclk_sel;
> +
> + clk = clk_register_mux_table(cpts->dev, refclk_np->name,
> + parent_names, num_parents,
> + 0, reg, 0, 0x1F, 0, mux_table, NULL);
> + if (IS_ERR(clk))
> + return PTR_ERR(clk);
> +
> + return of_clk_add_provider(refclk_np, of_clk_src_simple_get, clk);
Can you please use the clk_hw APIs instead?
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH 2/3] ASoC: simple-card: use devm_get_clk_from_child()
From: Kuninori Morimoto @ 2016-12-09 0:33 UTC (permalink / raw)
To: Stephen Boyd
Cc: Russell King - ARM Linux, Rob Herring, Linux-ALSA, Linux-DT,
Michael Turquette, Linux-Kernel, Mark Brown, linux-clk, Linux-ARM
In-Reply-To: <20161209002837.GE5423@codeaurora.org>
Hi Stephen
> > Documentation/devicetree/bindings/sound/simple-card.txt
> > explains 1st of_clk_get will be used as "if needed",
> > 2nd of_clk_get will be used as "not needed pattern".
> > 1st pattern will use specific clock, 2nd pattern will use
> > "cpu" or "codec" clock.
> > 2nd one was added by someone (I forgot), and many driver is
> > based on this feature.
> >
>
> Can you point to some dts file in the kernel that falls into the
> devm_get_clk_from_child(dev, dai_of_node, NULL) part?
How about this ?
linux/arch/arm/boot/dts/r8a7790-lager.dts :: rsnd_ak4643
^ permalink raw reply
* Re: [PATCH 2/3] ASoC: simple-card: use devm_get_clk_from_child()
From: Stephen Boyd @ 2016-12-09 0:28 UTC (permalink / raw)
To: Kuninori Morimoto
Cc: Russell King - ARM Linux, Rob Herring, Linux-ALSA, Linux-DT,
Michael Turquette, Linux-Kernel, Mark Brown,
linux-clk-u79uwXL29TY76Z2rM5mHXA, Linux-ARM
In-Reply-To: <877f7aymxu.wl%kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
On 12/09, Kuninori Morimoto wrote:
>
> Hi Stephen
>
> > > @@ -111,14 +112,13 @@ int asoc_simple_card_parse_clk(struct device_node *node,
> > > * or "system-clock-frequency = <xxx>"
> > > * or device's module clock.
> > > */
> > > - clk = of_clk_get(node, 0);
> > > + clk = devm_get_clk_from_child(dev, node, NULL);
> > > if (!IS_ERR(clk)) {
> > > simple_dai->sysclk = clk_get_rate(clk);
> > > - simple_dai->clk = clk;
> > > } else if (!of_property_read_u32(node, "system-clock-frequency", &val)) {
> > > simple_dai->sysclk = val;
> > > } else {
> > > - clk = of_clk_get(dai_of_node, 0);
> > > + clk = devm_get_clk_from_child(dev, dai_of_node, NULL);
> >
> >
> > I was confused for a minute about how the second of_clk_get()
> > call with the dai_link node could work. Is that documented
> > anywhere or used by anyone? It seems like it's at least another
> > child node of the sound node (which is dev here) so it seems ok.
>
> Documentation/devicetree/bindings/sound/simple-card.txt
> explains 1st of_clk_get will be used as "if needed",
> 2nd of_clk_get will be used as "not needed pattern".
> 1st pattern will use specific clock, 2nd pattern will use
> "cpu" or "codec" clock.
> 2nd one was added by someone (I forgot), and many driver is
> based on this feature.
>
Can you point to some dts file in the kernel that falls into the
devm_get_clk_from_child(dev, dai_of_node, NULL) part?
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 3/3] ASoC: simple-card-utils: enable clocks/clock-names/clock-ranges
From: Stephen Boyd @ 2016-12-09 0:26 UTC (permalink / raw)
To: Kuninori Morimoto
Cc: Russell King - ARM Linux, Rob Herring, Linux-ALSA, Linux-DT,
Michael Turquette, Linux-Kernel, Mark Brown, linux-clk, Linux-ARM
In-Reply-To: <874m2eymu3.wl%kuninori.morimoto.gx@renesas.com>
On 12/09, Kuninori Morimoto wrote:
>
> Hi Stephen
>
> > > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > >
> > > Current simple-card is supporting this style for clocks
> > >
> > > sound {
> > > ...
> > > simple-audio-card,cpu {
> > > sound-dai = <&xxx>;
> > > clocks = <&cpu_clock>;
> > > };
> > > simple-audio-card,codec {
> > > sound-dai = <&xxx>;
> > > clocks = <&codec_clock>;
> > > };
> > > };
> > >
> > > Now, it can support this style too, because we can use
> > > devm_get_clk_from_child() now.
> > >
> > > sound {
> > > ...
> > > clocks = <&cpu_clock>, <&codec_clock>;
> > > clock-names = "cpu", "codec";
> > > clock-ranges;
> > > ...
> > > simple-audio-card,cpu {
> > > sound-dai = <&xxx>;
> > > };
> > > simple-audio-card,codec {
> > > sound-dai = <&xxx>;
> > > };
> > > };
> > >
> > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> >
> > I don't see any reason why we need this patch though. The binding
> > works as is, so supporting different styles doesn't seem like a
> > good idea to me. Let's just keep what we have? Even if a sub-node
> > like cpu or codec gets more than one element in the clocks list
> > property, we can make that work by passing a clock-name then
> > based on some sort of other knowledge.
>
> OK, thanks. Let's skip this patch.
> But I believe this idea/method itself is not wrong (?)
>
Right it's not wrong, just seems confusing to have two methods.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH 0/3] clkdev: add devm_get_clk_from_child()
From: Kuninori Morimoto @ 2016-12-09 0:25 UTC (permalink / raw)
To: Stephen Boyd
Cc: Russell King - ARM Linux, Rob Herring, Linux-ALSA, Linux-DT,
Michael Turquette, Linux-Kernel, Mark Brown,
linux-clk-u79uwXL29TY76Z2rM5mHXA, Linux-ARM
In-Reply-To: <20161208220824.GM5423-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Hi Stephen, Mark
> > This is v5 of "clkdev: add devm_of_clk_get()", but new series.
> > I hope my understanding was correct with your idea.
>
> Yes this looks good. Given that we're so close to the merge
> window, perhaps I should just merge the first patch into clk-next
> and then it will be ready for anyone who wants to use it? The
> sound patches can be left up to others to handle.
OK thanks.
Mark, I think I should re-post 2nd patch (3rd will be dropped) after
merge window ? There will be no branch dependency
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 3/3] ASoC: simple-card-utils: enable clocks/clock-names/clock-ranges
From: Kuninori Morimoto @ 2016-12-09 0:22 UTC (permalink / raw)
To: Stephen Boyd
Cc: Russell King - ARM Linux, Rob Herring, Linux-ALSA, Linux-DT,
Michael Turquette, Linux-Kernel, Mark Brown,
linux-clk-u79uwXL29TY76Z2rM5mHXA, Linux-ARM
In-Reply-To: <20161208220901.GN5423-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Hi Stephen
> > From: Kuninori Morimoto <kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
> >
> > Current simple-card is supporting this style for clocks
> >
> > sound {
> > ...
> > simple-audio-card,cpu {
> > sound-dai = <&xxx>;
> > clocks = <&cpu_clock>;
> > };
> > simple-audio-card,codec {
> > sound-dai = <&xxx>;
> > clocks = <&codec_clock>;
> > };
> > };
> >
> > Now, it can support this style too, because we can use
> > devm_get_clk_from_child() now.
> >
> > sound {
> > ...
> > clocks = <&cpu_clock>, <&codec_clock>;
> > clock-names = "cpu", "codec";
> > clock-ranges;
> > ...
> > simple-audio-card,cpu {
> > sound-dai = <&xxx>;
> > };
> > simple-audio-card,codec {
> > sound-dai = <&xxx>;
> > };
> > };
> >
> > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
>
> I don't see any reason why we need this patch though. The binding
> works as is, so supporting different styles doesn't seem like a
> good idea to me. Let's just keep what we have? Even if a sub-node
> like cpu or codec gets more than one element in the clocks list
> property, we can make that work by passing a clock-name then
> based on some sort of other knowledge.
OK, thanks. Let's skip this patch.
But I believe this idea/method itself is not wrong (?)
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 3/3] ASoC: simple-card-utils: enable clocks/clock-names/clock-ranges
From: Kuninori Morimoto @ 2016-12-09 0:21 UTC (permalink / raw)
To: Stephen Boyd
Cc: Russell King - ARM Linux, Rob Herring, Linux-ALSA, Linux-DT,
Michael Turquette, Linux-Kernel, Mark Brown,
linux-clk-u79uwXL29TY76Z2rM5mHXA, Linux-ARM
In-Reply-To: <20161208220901.GN5423-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Hi Stephen
> > From: Kuninori Morimoto <kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
> >
> > Current simple-card is supporting this style for clocks
> >
> > sound {
> > ...
> > simple-audio-card,cpu {
> > sound-dai = <&xxx>;
> > clocks = <&cpu_clock>;
> > };
> > simple-audio-card,codec {
> > sound-dai = <&xxx>;
> > clocks = <&codec_clock>;
> > };
> > };
> >
> > Now, it can support this style too, because we can use
> > devm_get_clk_from_child() now.
> >
> > sound {
> > ...
> > clocks = <&cpu_clock>, <&codec_clock>;
> > clock-names = "cpu", "codec";
> > clock-ranges;
> > ...
> > simple-audio-card,cpu {
> > sound-dai = <&xxx>;
> > };
> > simple-audio-card,codec {
> > sound-dai = <&xxx>;
> > };
> > };
> >
> > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
>
> I don't see any reason why we need this patch though. The binding
> works as is, so supporting different styles doesn't seem like a
> good idea to me. Let's just keep what we have? Even if a sub-node
> like cpu or codec gets more than one element in the clocks list
> property, we can make that work by passing a clock-name then
> based on some sort of other knowledge.
OK, thanks. Let's skip this patch.
But I believe this idea itself is not wrong (?)
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/3] ASoC: simple-card: use devm_get_clk_from_child()
From: Kuninori Morimoto @ 2016-12-09 0:20 UTC (permalink / raw)
To: Stephen Boyd
Cc: Russell King - ARM Linux, Rob Herring, Linux-ALSA, Linux-DT,
Michael Turquette, Linux-Kernel, Mark Brown, linux-clk, Linux-ARM
In-Reply-To: <20161208220942.GO5423@codeaurora.org>
Hi Stephen
> > @@ -111,14 +112,13 @@ int asoc_simple_card_parse_clk(struct device_node *node,
> > * or "system-clock-frequency = <xxx>"
> > * or device's module clock.
> > */
> > - clk = of_clk_get(node, 0);
> > + clk = devm_get_clk_from_child(dev, node, NULL);
> > if (!IS_ERR(clk)) {
> > simple_dai->sysclk = clk_get_rate(clk);
> > - simple_dai->clk = clk;
> > } else if (!of_property_read_u32(node, "system-clock-frequency", &val)) {
> > simple_dai->sysclk = val;
> > } else {
> > - clk = of_clk_get(dai_of_node, 0);
> > + clk = devm_get_clk_from_child(dev, dai_of_node, NULL);
>
>
> I was confused for a minute about how the second of_clk_get()
> call with the dai_link node could work. Is that documented
> anywhere or used by anyone? It seems like it's at least another
> child node of the sound node (which is dev here) so it seems ok.
Documentation/devicetree/bindings/sound/simple-card.txt
explains 1st of_clk_get will be used as "if needed",
2nd of_clk_get will be used as "not needed pattern".
1st pattern will use specific clock, 2nd pattern will use
"cpu" or "codec" clock.
2nd one was added by someone (I forgot), and many driver is
based on this feature.
Best regards
---
Kuninori Morimoto
^ permalink raw reply
* Re: [PATCH] misc: eeprom: implement compatible DT probing
From: Linus Walleij @ 2016-12-08 23:32 UTC (permalink / raw)
To: Peter Rosin
Cc: Wolfram Sang, linux-i2c@vger.kernel.org,
devicetree@vger.kernel.org
In-Reply-To: <95d9c739-8a69-b990-2840-b5381d54a99d@axentia.se>
On Thu, Dec 8, 2016 at 7:23 PM, Peter Rosin <peda@axentia.se> wrote:
> On 2016-12-08 18:47, Linus Walleij wrote:
>> Before this patch, the following device tree node does not probe,
>> which might be considered a bug:
>>
>> eeprom@52 {
>> compatible = "atmel,at24c128";
>
> The way I read it, that should be "atmel,24c128", i.e. w/o the "at" prefix.
(...)
> and then lists the compatibles you have added to the match table (but you
> have this extra "at" prefix for the atmel manufacturer).
>
> The way I read the above, you are free to use any manufacturer and still
> have it work, and indeed, I have success with this node:
>
> eeprom@50 {
> compatible = "nxp,24c02";
> reg = <0x50>;
> pagesize = <16>;
> };
>
> I fear that your patch will regress this matching on the wildcard
> manufacturer. I haven't tested that though, but there are enough
> question marks anyway...
Bah I probably just screwed up syntactically and now worked around
a non-existing problem. I will try as you suggest, just "vendor,type"
and see if it works. It probably does.
Some days I feel just utterly stupid. Sorry for the fuzz.
Wolfram: ignore the patch for now.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 10/11] Document: dt: binding: imx: update doc for imx6sll
From: Stephen Boyd @ 2016-12-08 22:54 UTC (permalink / raw)
To: Bai Ping
Cc: shawnguo, mturquette, robh+dt, mark.rutland, linus.walleij,
kernel, fabio.estevam, daniel.lezcano, tglx, p.zabel, linux-clk,
devicetree, linux-gpio, linux-arm-kernel
In-Reply-To: <1480660774-25055-11-git-send-email-ping.bai@nxp.com>
On 12/02, Bai Ping wrote:
> Add necessary document update for i.MX6SLL support.
>
> Signed-off-by: Bai Ping <ping.bai@nxp.com>
> ---
> .../devicetree/bindings/clock/imx6sll-clock.txt | 13 ++++++++
> .../bindings/pinctrl/fsl,imx6sll-pinctrl.txt | 37 ++++++++++++++++++++++
Please split the bindings into different patches and put them
closer to the drivers that use them in the series.
> 2 files changed, 50 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/imx6sll-clock.txt
> create mode 100644 Documentation/devicetree/bindings/pinctrl/fsl,imx6sll-pinctrl.txt
>
> diff --git a/Documentation/devicetree/bindings/clock/imx6sll-clock.txt b/Documentation/devicetree/bindings/clock/imx6sll-clock.txt
> new file mode 100644
> index 0000000..4f52efa
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/imx6sll-clock.txt
> @@ -0,0 +1,13 @@
> +* Clock bindings for Freescale i.MX6 UltraLite
> +
> +Required properties:
> +- compatible: Should be "fsl,imx6sll-ccm"
> +- reg: Address and length of the register set
> +- #clock-cells: Should be <1>
> +- clocks: list of clock specifiers, must contain an entry for each required
> + entry in clock-names
> +- clock-names: should include entries "ckil", "osc", "ipp_di0" and "ipp_di1"
> +
> +The clock consumer should specify the desired clock by having the clock
> +ID in its "clocks" phandle cell. See include/dt-bindings/clock/imx6sll-clock.h
> +for the full list of i.MX6 SLL clock IDs.
Can you add an example node here?
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH 03/11] driver: clk: imx: Add clock driver for imx6sll
From: Stephen Boyd @ 2016-12-08 22:52 UTC (permalink / raw)
To: Bai Ping
Cc: shawnguo, mturquette, robh+dt, mark.rutland, linus.walleij,
kernel, fabio.estevam, daniel.lezcano, tglx, p.zabel, linux-clk,
devicetree, linux-gpio, linux-arm-kernel
In-Reply-To: <1480660774-25055-4-git-send-email-ping.bai@nxp.com>
On 12/02, Bai Ping wrote:
> diff --git a/drivers/clk/imx/clk-imx6sll.c b/drivers/clk/imx/clk-imx6sll.c
> new file mode 100644
> index 0000000..c5219e1
> --- /dev/null
> +++ b/drivers/clk/imx/clk-imx6sll.c
> @@ -0,0 +1,366 @@
> +/*
> + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> + *
> + * The code contained herein is licensed under the GNU General Public
> + * License. You may obtain a copy of the GNU General Public License
> + * Version 2 or later at the following locations:
> + *
> + * http://www.opensource.org/licenses/gpl-license.html
> + * http://www.gnu.org/copyleft/gpl.html
> + */
> +
> +#include <dt-bindings/clock/imx6sll-clock.h>
> +#include <linux/clk.h>
> +#include <linux/clkdev.h>
Is this include used?
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/types.h>
Is there an include of clk_provider.h missing?
> +
> +#include "clk.h"
> +
> +#define BM_CCM_CCDR_MMDC_CH0_MASK (0x2 << 16)
> +#define CCDR 0x4
> +
> +static const char *pll_bypass_src_sels[] = { "osc", "dummy", };
const char * const? For all of these names.
> +static const char *pll1_bypass_sels[] = { "pll1", "pll1_bypass_src", };
> +static const char *pll2_bypass_sels[] = { "pll2", "pll2_bypass_src", };
> +static const char *pll3_bypass_sels[] = { "pll3", "pll3_bypass_src", };
> +static const char *pll4_bypass_sels[] = { "pll4", "pll4_bypass_src", };
> +static const char *pll5_bypass_sels[] = { "pll5", "pll5_bypass_src", };
> +static const char *pll6_bypass_sels[] = { "pll6", "pll6_bypass_src", };
> +static const char *pll7_bypass_sels[] = { "pll7", "pll7_bypass_src", };
> +static const char *step_sels[] = { "osc", "pll2_pfd2_396m", };
> +static const char *pll1_sw_sels[] = { "pll1_sys", "step", };
> +static const char *axi_alt_sels[] = { "pll2_pfd2_396m", "pll3_pfd1_540m", };
> +static const char *axi_sels[] = {"periph", "axi_alt_sel", };
> +static const char *periph_pre_sels[] = { "pll2_bus", "pll2_pfd2_396m", "pll2_pfd0_352m", "pll2_198m", };
> +static const char *periph2_pre_sels[] = { "pll2_bus", "pll2_pfd2_396m", "pll2_pfd0_352m", "pll4_audio_div", };
> +static const char *periph_clk2_sels[] = { "pll3_usb_otg", "osc", "osc", };
> +static const char *periph2_clk2_sels[] = { "pll3_usb_otg", "osc", };
> +static const char *periph_sels[] = { "periph_pre", "periph_clk2", };
> +static const char *periph2_sels[] = { "periph2_pre", "periph2_clk2", };
> +static const char *usdhc_sels[] = { "pll2_pfd2_396m", "pll2_pfd0_352m", };
> +static const char *ssi_sels[] = {"pll3_pfd2_508m", "pll3_pfd3_454m", "pll4_audio_div", "dummy",};
> +static const char *spdif_sels[] = { "pll4_audio_div", "pll3_pfd2_508m", "pll5_video_div", "pll3_usb_otg", };
> +static const char *ldb_di0_div_sels[] = { "ldb_di0_div_3_5", "ldb_di0_div_7", };
> +static const char *ldb_di1_div_sels[] = { "ldb_di1_div_3_5", "ldb_di1_div_7", };
> +static const char *ldb_di0_sels[] = { "pll5_video_div", "pll2_pfd0_352m", "pll2_pfd2_396m", "pll2_pfd3_594m", "pll2_pfd1_594m", "pll3_pfd3_454m", };
> +static const char *ldb_di1_sels[] = { "pll3_usb_otg", "pll2_pfd0_352m", "pll2_pfd2_396m", "pll2_bus", "pll3_pfd3_454m", "pll3_pfd2_508m", };
> +static const char *lcdif_pre_sels[] = { "pll2_bus", "pll3_pfd3_454m", "pll5_video_div", "pll2_pfd0_352m", "pll2_pfd1_594m", "pll3_pfd1_540m", };
> +static const char *ecspi_sels[] = { "pll3_60m", "osc", };
> +static const char *uart_sels[] = { "pll3_80m", "osc", };
> +static const char *perclk_sels[] = { "ipg", "osc", };
> +static const char *lcdif_sels[] = { "lcdif_podf", "ipp_di0", "ipp_di1", "ldb_di0", "ldb_di1", };
> +
> +static const char *epdc_pre_sels[] = { "pll2_bus", "pll3_usb_otg", "pll5_video_div", "pll2_pfd0_352m", "pll2_pfd2_396m", "pll3_pfd2_508m", };
> +static const char *epdc_sels[] = { "epdc_podf", "ipp_di0", "ipp_di1", "ldb_di0", "ldb_di1", };
> +
> +static struct clk *clks[IMX6SLL_CLK_END];
> +static struct clk_onecell_data clk_data;
> +
> +static int const clks_init_on[] __initconst = {
static const int is more preferred style.
> + IMX6SLL_CLK_AIPSTZ1, IMX6SLL_CLK_AIPSTZ2,
> + IMX6SLL_CLK_OCRAM, IMX6SLL_CLK_ARM, IMX6SLL_CLK_ROM,
> + IMX6SLL_CLK_MMDC_P0_FAST, IMX6SLL_CLK_MMDC_P0_IPG,
> +};
> +
> +static struct clk_div_table post_div_table[] = {
const?
> + { .val = 2, .div = 1, },
> + { .val = 1, .div = 2, },
> + { .val = 0, .div = 4, },
> + { }
> +};
> +
> +static struct clk_div_table video_div_table[] = {
const?
> + { .val = 0, .div = 1, },
> + { .val = 1, .div = 2, },
> + { .val = 2, .div = 1, },
> + { .val = 3, .div = 4, },
> + { }
> +};
> +
> +static u32 share_count_audio;
> +static u32 share_count_ssi1;
> +static u32 share_count_ssi2;
> +static u32 share_count_ssi3;
> +
> +static void __init imx6sll_clocks_init(struct device_node *ccm_node)
> +{
> + struct device_node *np;
> + void __iomem *base;
> + int i;
> +
> + clks[IMX6SLL_CLK_DUMMY] = imx_clk_fixed("dummy", 0);
> +
> + clks[IMX6SLL_CLK_CKIL] = of_clk_get_by_name(ccm_node, "ckil");
> + clks[IMX6SLL_CLK_OSC] = of_clk_get_by_name(ccm_node, "osc");
> +
> + /* ipp_di clock is external input */
> + clks[IMX6SLL_CLK_IPP_DI0] = of_clk_get_by_name(ccm_node, "ipp_di0");
> + clks[IMX6SLL_CLK_IPP_DI1] = of_clk_get_by_name(ccm_node, "ipp_di1");
> +
> + np = of_find_compatible_node(NULL, NULL, "fsl,imx6sll-anatop");
> + base = of_iomap(np, 0);
> + WARN_ON(!base);
> +
> + clks[IMX6SLL_PLL1_BYPASS_SRC] = imx_clk_mux("pll1_bypass_src", base + 0x00, 14, 1, pll_bypass_src_sels, ARRAY_SIZE(pll_bypass_src_sels));
> + clks[IMX6SLL_PLL2_BYPASS_SRC] = imx_clk_mux("pll2_bypass_src", base + 0x30, 14, 1, pll_bypass_src_sels, ARRAY_SIZE(pll_bypass_src_sels));
> + clks[IMX6SLL_PLL3_BYPASS_SRC] = imx_clk_mux("pll3_bypass_src", base + 0x10, 14, 1, pll_bypass_src_sels, ARRAY_SIZE(pll_bypass_src_sels));
> + clks[IMX6SLL_PLL4_BYPASS_SRC] = imx_clk_mux("pll4_bypass_src", base + 0x70, 14, 1, pll_bypass_src_sels, ARRAY_SIZE(pll_bypass_src_sels));
> + clks[IMX6SLL_PLL5_BYPASS_SRC] = imx_clk_mux("pll5_bypass_src", base + 0xa0, 14, 1, pll_bypass_src_sels, ARRAY_SIZE(pll_bypass_src_sels));
> + clks[IMX6SLL_PLL6_BYPASS_SRC] = imx_clk_mux("pll6_bypass_src", base + 0xe0, 14, 1, pll_bypass_src_sels, ARRAY_SIZE(pll_bypass_src_sels));
> + clks[IMX6SLL_PLL7_BYPASS_SRC] = imx_clk_mux("pll7_bypass_src", base + 0x20, 14, 1, pll_bypass_src_sels, ARRAY_SIZE(pll_bypass_src_sels));
> +
> + clks[IMX6SLL_CLK_PLL1] = imx_clk_pllv3(IMX_PLLV3_SYS, "pll1", "pll1_bypass_src", base + 0x00, 0x7f);
> + clks[IMX6SLL_CLK_PLL2] = imx_clk_pllv3(IMX_PLLV3_GENERIC, "pll2", "pll2_bypass_src", base + 0x30, 0x1);
> + clks[IMX6SLL_CLK_PLL3] = imx_clk_pllv3(IMX_PLLV3_USB, "pll3", "pll3_bypass_src", base + 0x10, 0x3);
> + clks[IMX6SLL_CLK_PLL4] = imx_clk_pllv3(IMX_PLLV3_AV, "pll4", "pll4_bypass_src", base + 0x70, 0x7f);
> + clks[IMX6SLL_CLK_PLL5] = imx_clk_pllv3(IMX_PLLV3_AV, "pll5", "pll5_bypass_src", base + 0xa0, 0x7f);
> + clks[IMX6SLL_CLK_PLL6] = imx_clk_pllv3(IMX_PLLV3_ENET, "pll6", "pll6_bypass_src", base + 0xe0, 0x3);
> + clks[IMX6SLL_CLK_PLL7] = imx_clk_pllv3(IMX_PLLV3_USB, "pll7", "pll7_bypass_src", base + 0x20, 0x3);
> +
> + clks[IMX6SLL_PLL1_BYPASS] = imx_clk_mux_flags("pll1_bypass", base + 0x00, 16, 1, pll1_bypass_sels, ARRAY_SIZE(pll1_bypass_sels), CLK_SET_RATE_PARENT);
> + clks[IMX6SLL_PLL2_BYPASS] = imx_clk_mux_flags("pll2_bypass", base + 0x30, 16, 1, pll2_bypass_sels, ARRAY_SIZE(pll2_bypass_sels), CLK_SET_RATE_PARENT);
> + clks[IMX6SLL_PLL3_BYPASS] = imx_clk_mux_flags("pll3_bypass", base + 0x10, 16, 1, pll3_bypass_sels, ARRAY_SIZE(pll3_bypass_sels), CLK_SET_RATE_PARENT);
> + clks[IMX6SLL_PLL4_BYPASS] = imx_clk_mux_flags("pll4_bypass", base + 0x70, 16, 1, pll4_bypass_sels, ARRAY_SIZE(pll4_bypass_sels), CLK_SET_RATE_PARENT);
> + clks[IMX6SLL_PLL5_BYPASS] = imx_clk_mux_flags("pll5_bypass", base + 0xa0, 16, 1, pll5_bypass_sels, ARRAY_SIZE(pll5_bypass_sels), CLK_SET_RATE_PARENT);
> + clks[IMX6SLL_PLL6_BYPASS] = imx_clk_mux_flags("pll6_bypass", base + 0xe0, 16, 1, pll6_bypass_sels, ARRAY_SIZE(pll6_bypass_sels), CLK_SET_RATE_PARENT);
> + clks[IMX6SLL_PLL7_BYPASS] = imx_clk_mux_flags("pll7_bypass", base + 0x20, 16, 1, pll7_bypass_sels, ARRAY_SIZE(pll7_bypass_sels), CLK_SET_RATE_PARENT);
> +
> + /* Do not bypass PLLs initially */
> + clk_set_parent(clks[IMX6SLL_PLL1_BYPASS], clks[IMX6SLL_CLK_PLL1]);
> + clk_set_parent(clks[IMX6SLL_PLL2_BYPASS], clks[IMX6SLL_CLK_PLL2]);
> + clk_set_parent(clks[IMX6SLL_PLL3_BYPASS], clks[IMX6SLL_CLK_PLL3]);
> + clk_set_parent(clks[IMX6SLL_PLL4_BYPASS], clks[IMX6SLL_CLK_PLL4]);
> + clk_set_parent(clks[IMX6SLL_PLL5_BYPASS], clks[IMX6SLL_CLK_PLL5]);
> + clk_set_parent(clks[IMX6SLL_PLL6_BYPASS], clks[IMX6SLL_CLK_PLL6]);
> + clk_set_parent(clks[IMX6SLL_PLL7_BYPASS], clks[IMX6SLL_CLK_PLL7]);
Can we just put raw register writes here instead? I'd prefer we
didn't use clk consumer APIs to do things to the clk tree from
the providers. The problem there being that:
1) We're trying to move away from using consumer APIs in
provider drivers. It's ok if they're used during probe, but
inside clk_ops is not preferred.
2) Even if you have a clk pointer, it may be "orphaned" at the
time of registration and so calling the APIs here works now but
eventually we may want to return an EPROBE_DEFER error in that
case and this may block that effort.
I suppose if this is the only clk driver on this machine then
this last point isn't a concern and things are probably ok here.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH 04/16] drivers/fsi: Add fsi master definition
From: Christopher Bostic @ 2016-12-08 22:49 UTC (permalink / raw)
To: Greg KH
Cc: Rob Herring, Mark Rutland, linux-I+IVW8TIWO2tmTQ+vhA3Yw,
sre-DgEjT+Ai2ygdnm+yROfE0A, Michael Turquette,
geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ,
Open List OF Flattened dev tree bindings,
Moderated list: ARM PORT, Jeremy Kerr, Joel Stanley,
Linux open list, Andrew Jeffery, Alistair Popple,
Benjamin Herrenschmidt, Chris Bostic
In-Reply-To: <20161207090610.GB14742-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
On Wed, Dec 7, 2016 at 3:06 AM, Greg KH <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org> wrote:
> On Tue, Dec 06, 2016 at 08:09:30PM -0600, Chris Bostic wrote:
>> From: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
>>
>> Signed-off-by: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
>> Signed-off-by: Chris Bostic <cbostic-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
>> ---
>> drivers/fsi/fsi-core.c | 20 ++++++++++++++++++++
>> drivers/fsi/fsi-master.h | 37 +++++++++++++++++++++++++++++++++++++
>> 2 files changed, 57 insertions(+)
>> create mode 100644 drivers/fsi/fsi-master.h
>>
>> diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
>> index 3d55bd5..ce9428d 100644
>> --- a/drivers/fsi/fsi-core.c
>> +++ b/drivers/fsi/fsi-core.c
>> @@ -17,6 +17,26 @@
>> #include <linux/fsi.h>
>> #include <linux/module.h>
>>
>> +#include "fsi-master.h"
>> +
>> +static atomic_t master_idx = ATOMIC_INIT(-1);
>
> You don't really want/need an atomic variable, please use the simple ida
> interface instead.
Greg,
Will make the change to simple ida interface.
Thanks for your feedback,
Chris
>
> thanks,
>
> greg k-h
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/3] ASoC: simple-card: use devm_get_clk_from_child()
From: Stephen Boyd @ 2016-12-08 22:09 UTC (permalink / raw)
To: Kuninori Morimoto
Cc: Russell King - ARM Linux, Rob Herring, Linux-ALSA, Linux-DT,
Michael Turquette, Linux-Kernel, Mark Brown, linux-clk, Linux-ARM
In-Reply-To: <871sxnvtkp.wl%kuninori.morimoto.gx@renesas.com>
On 12/05, Kuninori Morimoto wrote:
> diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c
> index cf02625..4924575 100644
> --- a/sound/soc/generic/simple-card-utils.c
> +++ b/sound/soc/generic/simple-card-utils.c
> @@ -98,7 +98,8 @@ int asoc_simple_card_parse_card_name(struct snd_soc_card *card,
> }
> EXPORT_SYMBOL_GPL(asoc_simple_card_parse_card_name);
>
> -int asoc_simple_card_parse_clk(struct device_node *node,
> +int asoc_simple_card_parse_clk(struct device *dev,
> + struct device_node *node,
> struct device_node *dai_of_node,
> struct asoc_simple_dai *simple_dai)
> {
> @@ -111,14 +112,13 @@ int asoc_simple_card_parse_clk(struct device_node *node,
> * or "system-clock-frequency = <xxx>"
> * or device's module clock.
> */
> - clk = of_clk_get(node, 0);
> + clk = devm_get_clk_from_child(dev, node, NULL);
> if (!IS_ERR(clk)) {
> simple_dai->sysclk = clk_get_rate(clk);
> - simple_dai->clk = clk;
> } else if (!of_property_read_u32(node, "system-clock-frequency", &val)) {
> simple_dai->sysclk = val;
> } else {
> - clk = of_clk_get(dai_of_node, 0);
> + clk = devm_get_clk_from_child(dev, dai_of_node, NULL);
I was confused for a minute about how the second of_clk_get()
call with the dai_link node could work. Is that documented
anywhere or used by anyone? It seems like it's at least another
child node of the sound node (which is dev here) so it seems ok.
> if (!IS_ERR(clk))
> simple_dai->sysclk = clk_get_rate(clk);
> }
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH 3/3] ASoC: simple-card-utils: enable clocks/clock-names/clock-ranges
From: Stephen Boyd @ 2016-12-08 22:09 UTC (permalink / raw)
To: Kuninori Morimoto
Cc: Russell King - ARM Linux, Rob Herring, Linux-ALSA, Linux-DT,
Michael Turquette, Linux-Kernel, Mark Brown, linux-clk, Linux-ARM
In-Reply-To: <87zikbuezr.wl%kuninori.morimoto.gx@renesas.com>
On 12/05, Kuninori Morimoto wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
>
> Current simple-card is supporting this style for clocks
>
> sound {
> ...
> simple-audio-card,cpu {
> sound-dai = <&xxx>;
> clocks = <&cpu_clock>;
> };
> simple-audio-card,codec {
> sound-dai = <&xxx>;
> clocks = <&codec_clock>;
> };
> };
>
> Now, it can support this style too, because we can use
> devm_get_clk_from_child() now.
>
> sound {
> ...
> clocks = <&cpu_clock>, <&codec_clock>;
> clock-names = "cpu", "codec";
> clock-ranges;
> ...
> simple-audio-card,cpu {
> sound-dai = <&xxx>;
> };
> simple-audio-card,codec {
> sound-dai = <&xxx>;
> };
> };
>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
I don't see any reason why we need this patch though. The binding
works as is, so supporting different styles doesn't seem like a
good idea to me. Let's just keep what we have? Even if a sub-node
like cpu or codec gets more than one element in the clocks list
property, we can make that work by passing a clock-name then
based on some sort of other knowledge.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH 0/3] clkdev: add devm_get_clk_from_child()
From: Stephen Boyd @ 2016-12-08 22:08 UTC (permalink / raw)
To: Kuninori Morimoto
Cc: Russell King - ARM Linux, Rob Herring, Linux-ALSA, Linux-DT,
Michael Turquette, Linux-Kernel, Mark Brown, linux-clk, Linux-ARM
In-Reply-To: <874m2jvtmw.wl%kuninori.morimoto.gx@renesas.com>
On 12/05, Kuninori Morimoto wrote:
>
> Hi Stephen
>
> This is v5 of "clkdev: add devm_of_clk_get()", but new series.
> I hope my understanding was correct with your idea.
Yes this looks good. Given that we're so close to the merge
window, perhaps I should just merge the first patch into clk-next
and then it will be ready for anyone who wants to use it? The
sound patches can be left up to others to handle.
>
> Kuninori Morimoto (3):
> 1) clkdev: add devm_get_clk_from_child()
> 2) ASoC: simple-card: use devm_get_clk_from_child()
> 3) ASoC: simple-card-utils: enable clocks/clock-names/clock-ranges
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH 3/3] clk: keystone: Add sci-clk driver support
From: Stephen Boyd @ 2016-12-08 21:10 UTC (permalink / raw)
To: Tero Kristo
Cc: linux-clk, mturquette, ssantosh, nm, linux-arm-kernel, devicetree
In-Reply-To: <cb608729-b617-0eab-5ddf-d78ff47b237e@ti.com>
On 12/08, Tero Kristo wrote:
> On 08/12/16 02:13, Stephen Boyd wrote:
> >On 10/21, Tero Kristo wrote:
> >>diff --git a/drivers/clk/keystone/sci-clk.c b/drivers/clk/keystone/sci-clk.c
> >>new file mode 100644
> >>index 0000000..f6af5bd
> >>--- /dev/null
> >>+++ b/drivers/clk/keystone/sci-clk.c
>
> >
> >>+
> >>+ handle = devm_ti_sci_get_handle(dev);
> >>+ if (IS_ERR(handle))
> >>+ return PTR_ERR(handle);
> >>+
> >>+ provider = devm_kzalloc(dev, sizeof(*provider), GFP_KERNEL);
> >>+ if (!provider)
> >>+ return -ENOMEM;
> >>+
> >>+ provider->clocks = data;
> >>+
> >>+ provider->sci = handle;
> >>+ provider->ops = &handle->ops.clk_ops;
> >>+ provider->dev = dev;
> >>+
> >>+ ti_sci_init_clocks(provider);
> >
> >And if this fails?
>
> Yea this is kind of controversial. ti_sci_init_clocks() can fail if
> any of the clocks registered will fail. I decided to have it this
> way so that at least some clocks might work in failure cause, and
> you might have a booting device instead of total lock-up.
>
> Obviously it could be done so that if any clock fails, we would
> de-register all clocks at that point, but personally I think this is
> a worse option.
>
> ti_sci_init_clocks could probably be modified to continue
> registering clocks when a single clock fails though. Currently it
> aborts at first failure.
>
That sounds like a better approach if we don't care about
failures to register a clock. Returning a value from a function
and not using it isn't really a great design.
I worry that if we start returning errors from clk_hw_register()
that something will go wrong though, so really I don't know why
we want to ignore errors at all. Just for debugging a boot hang?
Can't we use early console to at least see that this driver is
failing to probe and debug that way?
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH 2/2] clk: zte: add audio clocks for zx296718
From: Stephen Boyd @ 2016-12-08 21:04 UTC (permalink / raw)
To: Shawn Guo
Cc: Michael Turquette, Rob Herring, Mark Rutland, Baoyou Xie, Jun Nie,
linux-clk-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Shawn Guo
In-Reply-To: <1481189157-8995-2-git-send-email-shawnguo-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
On 12/08, Shawn Guo wrote:
> +
> +static int __init audio_clocks_init(struct device_node *np)
> +{
> + void __iomem *reg_base;
> + int i, ret;
> +
> + reg_base = of_iomap(np, 0);
> + if (!reg_base) {
> + pr_err("%s: Unable to map audio clk base\n", __func__);
> + return -ENXIO;
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(audio_mux_clk); i++) {
> + if (audio_mux_clk[i].id)
> + audio_hw_onecell_data.hws[audio_mux_clk[i].id] =
> + &audio_mux_clk[i].mux.hw;
> +
> + audio_mux_clk[i].mux.reg += (u64)reg_base;
> + ret = clk_hw_register(NULL, &audio_mux_clk[i].mux.hw);
> + if (ret) {
> + pr_warn("audio clk %s init error!\n",
> + audio_mux_clk[i].mux.hw.init->name);
> + }
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(audio_adiv_clk); i++) {
> + if (audio_adiv_clk[i].id)
> + audio_hw_onecell_data.hws[audio_adiv_clk[i].id] =
> + &audio_adiv_clk[i].hw;
> +
> + audio_adiv_clk[i].reg_base += (u64)reg_base;
> + ret = clk_hw_register(NULL, &audio_adiv_clk[i].hw);
> + if (ret) {
> + pr_warn("audio clk %s init error!\n",
> + audio_adiv_clk[i].hw.init->name);
> + }
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(audio_div_clk); i++) {
> + if (audio_div_clk[i].id)
> + audio_hw_onecell_data.hws[audio_div_clk[i].id] =
> + &audio_div_clk[i].div.hw;
> +
> + audio_div_clk[i].div.reg += (u64)reg_base;
> + ret = clk_hw_register(NULL, &audio_div_clk[i].div.hw);
> + if (ret) {
> + pr_warn("audio clk %s init error!\n",
> + audio_div_clk[i].div.hw.init->name);
> + }
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(audio_gate_clk); i++) {
> + if (audio_gate_clk[i].id)
> + audio_hw_onecell_data.hws[audio_gate_clk[i].id] =
> + &audio_gate_clk[i].gate.hw;
> +
> + audio_gate_clk[i].gate.reg += (u64)reg_base;
> + ret = clk_hw_register(NULL, &audio_gate_clk[i].gate.hw);
> + if (ret) {
> + pr_warn("audio clk %s init error!\n",
> + audio_gate_clk[i].gate.hw.init->name);
> + }
> + }
> +
> + if (of_clk_add_hw_provider(np, of_clk_hw_onecell_get, &audio_hw_onecell_data))
> + panic("could not register clk provider\n");
Why don't we return error? We returned errors before if we
couldn't map the ioregion.
> + pr_info("audio-clk init over, nr:%d\n", AUDIO_NR_CLKS);
debug noise?
> +
> + return 0;
> +}
> +
> static const struct of_device_id zx_clkc_match_table[] = {
> { .compatible = "zte,zx296718-topcrm", .data = &top_clocks_init },
> { .compatible = "zte,zx296718-lsp0crm", .data = &lsp0_clocks_init },
> { .compatible = "zte,zx296718-lsp1crm", .data = &lsp1_clocks_init },
> + { .compatible = "zte,zx296718-audiocrm", .data = &audio_clocks_init },
> { }
> };
>
> diff --git a/drivers/clk/zte/clk.c b/drivers/clk/zte/clk.c
> index c4c1251bc1e7..ea97024b37aa 100644
> --- a/drivers/clk/zte/clk.c
> +++ b/drivers/clk/zte/clk.c
> @@ -9,6 +9,7 @@
>
> #include <linux/clk-provider.h>
> #include <linux/err.h>
> +#include <linux/gcd.h>
> #include <linux/io.h>
> #include <linux/iopoll.h>
> #include <linux/slab.h>
> @@ -310,3 +311,151 @@ struct clk *clk_register_zx_audio(const char *name,
>
> return clk;
> }
> +
> +#define CLK_AUDIO_DIV_FRAC BIT(0)
> +#define CLK_AUDIO_DIV_INT BIT(1)
> +#define CLK_AUDIO_DIV_UNCOMMON BIT(1)
> +
> +#define CLK_AUDIO_DIV_FRAC_NSHIFT 16
> +#define CLK_AUDIO_DIV_INT_FRAC_RE BIT(16)
> +#define CLK_AUDIO_DIV_INT_FRAC_MAX (0xffff)
> +#define CLK_AUDIO_DIV_INT_FRAC_MIN (0x2)
> +#define CLK_AUDIO_DIV_INT_INT_SHIFT 24
> +#define CLK_AUDIO_DIV_INT_INT_WIDTH 4
> +
> +#define to_clk_zx_audio_div(_hw) container_of(_hw, struct clk_zx_audio_divider, hw)
> +
> +static unsigned long audio_calc_rate(struct clk_zx_audio_divider *audio_div,
> + u32 reg_frac, u32 reg_int,
> + unsigned long parent_rate)
> +{
> + unsigned long rate, m, n;
> +
> + if (audio_div->table) {
> + const struct zx_clk_audio_div_table *divt = audio_div->table;
> +
> + for (; divt->rate; divt++) {
> + if ((divt->int_reg == reg_int) && (divt->frac_reg == reg_frac))
Please remove extra parenthesis here.
> + return divt->rate;
> + }
> + }
> + if (audio_div->table)
> + pr_warn("cannot found the config(int_reg:0x%x, frac_reg:0x%x) in table, we will caculate it\n",
> + reg_int, reg_frac);
> +
> + m = reg_frac & 0xffff;
> + n = (reg_frac >> 16) & 0xffff;
> +
> + m = (reg_int & 0xffff) * n + m;
> + rate = (parent_rate * n) / m;
> +
> + return rate;
> +}
> +
> +static void audio_calc_reg(struct clk_zx_audio_divider *audio_div,
> + struct zx_clk_audio_div_table *div_table,
> + unsigned long rate, unsigned long parent_rate)
> +{
> + unsigned int reg_int, reg_frac;
> + unsigned long m, n, div;
> +
> + if (audio_div->table) {
> + const struct zx_clk_audio_div_table *divt = audio_div->table;
> +
> + for (; divt->rate; divt++) {
> + if (divt->rate == rate) {
> + div_table->rate = divt->rate;
> + div_table->int_reg = divt->int_reg;
> + div_table->frac_reg = divt->frac_reg;
> + return;
> + }
> + }
> + }
> + if (audio_div->table)
> + pr_warn("cannot found the rate(%ld) in table, we will caculate the config\n",
> + rate);
> +
> + reg_int = parent_rate / rate;
> +
> + if (reg_int > CLK_AUDIO_DIV_INT_FRAC_MAX)
> + reg_int = CLK_AUDIO_DIV_INT_FRAC_MAX;
> + else if (reg_int < CLK_AUDIO_DIV_INT_FRAC_MIN)
> + reg_int = 0;
> + m = parent_rate - rate * reg_int;
> + n = rate;
> +
> + div = gcd(m, n);
> + m = m / div;
> + n = n / div;
> +
> + if ((m >> 16) || (n >> 16)) {
> + if (m > n) {
> + n = n * 0xffff / m;
> + m = 0xffff;
> + } else {
> + m = m * 0xffff / n;
> + n = 0xffff;
> + }
> + }
> + reg_frac = m | (n << 16);
> +
> + div_table->rate = (ulong)(parent_rate * n) / ((ulong)reg_int * n + m);
Please don't use ulong, use unsigned long. Also consider using
local variables so the line isn't overly long.
> + div_table->int_reg = reg_int;
> + div_table->frac_reg = reg_frac;
> +}
[...]
> +
> +static int zx_audio_div_set_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long parent_rate)
> +{
> + struct clk_zx_audio_divider *zx_audio_div = to_clk_zx_audio_div(hw);
> + struct zx_clk_audio_div_table divt;
> + unsigned int val;
> +
> + audio_calc_reg(zx_audio_div, &divt, rate, parent_rate);
> + if (divt.rate != rate)
> + pr_info("the real rate is:%ld", divt.rate);
Debug noise?
> +
> + writel_relaxed(divt.frac_reg, zx_audio_div->reg_base);
> +
> + val = readl_relaxed(zx_audio_div->reg_base + 0x4);
> + val &= ~0xffff;
> + val |= divt.int_reg | CLK_AUDIO_DIV_INT_FRAC_RE;
> + writel_relaxed(val, zx_audio_div->reg_base + 0x4);
> +
> + mdelay(1);
> +
> + val = readl_relaxed(zx_audio_div->reg_base + 0x4);
> + val &= ~CLK_AUDIO_DIV_INT_FRAC_RE;
> + writel_relaxed(val, zx_audio_div->reg_base + 0x4);
> +
> + return 0;
> +}
> +
> +const struct clk_ops zx_audio_div_ops = {
> + .recalc_rate = zx_audio_div_recalc_rate,
> + .round_rate = zx_audio_div_round_rate,
> + .set_rate = zx_audio_div_set_rate,
> +};
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 08/16] drivers/fsi: Add crc4 helpers
From: Christopher Bostic @ 2016-12-08 19:43 UTC (permalink / raw)
To: Jeremy Kerr
Cc: Greg KH, Rob Herring, Mark Rutland, linux-I+IVW8TIWO2tmTQ+vhA3Yw,
sre-DgEjT+Ai2ygdnm+yROfE0A, Michael Turquette,
geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ,
Open List OF Flattened dev tree bindings,
Moderated list: ARM PORT, Joel Stanley, Linux open list,
Andrew Jeffery, Alistair Popple, Benjamin Herrenschmidt,
Chris Bostic
In-Reply-To: <81146f25-05fa-70f9-e8ff-49c17aede8f2-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
On Wed, Dec 7, 2016 at 5:33 PM, Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org> wrote:
> Hi Greg,
>
>> Why not just create lib/crc4.c with these functions, like the other crc
>> functions in the kernel?
>
> Two (bad) reasons:
>
> - The crc4 implementation here is pretty specific to the FSI
> usage (only supporting 4-bit-sized chunks), to keep the math & lookup
> table simple
>
> - I'm lazy
>
> So yes, we should spend the effort now to make this generic enough for
> a lib/crc4.c. Would we want to support different values for the
> polynomial?
>
> Chris: do you want me to to that, or will you?
Hi Jeremy,
I'll take this one. Will implement as per Greg's suggestions.
Thanks,
Chris
>
> Cheers,
>
>
> Jeremy
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 3/6] net: ethernet: ti: cpts: add support of cpts HW_TS_PUSH
From: Grygorii Strashko @ 2016-12-08 19:04 UTC (permalink / raw)
To: Richard Cochran
Cc: David S. Miller, netdev-u79uwXL29TY76Z2rM5mHXA, Mugunthan V N,
Sekhar Nori, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-omap-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
devicetree-u79uwXL29TY76Z2rM5mHXA, Murali Karicheri, Wingman Kwok
In-Reply-To: <20161203232130.GA17944@netboy>
On 12/03/2016 05:21 PM, Richard Cochran wrote:
> On Mon, Nov 28, 2016 at 05:04:25PM -0600, Grygorii Strashko wrote:
>> This also change overflow polling period when HW_TS_PUSH feature is
>> enabled - overflow check work will be scheduled more often (every
>> 200ms) for proper HW_TS_PUSH events reporting.
>
> For proper reporting, you should make use of the interrupt. The small
> fifo (16 iirc) could very well overflow in 200 ms. The interrupt
> handler should read out the entire fifo at each interrupt.
>
huh. Seems this is not really good idea, because MISC Irq will be
triggered for *any* CPTS event and there is no way to enable it just for
HW_TS_PUSH. So, this doesn't work will with current code for RX/TX timestamping
(which uses polling mode). + runtime overhead in net RX/TX caused by
triggering more interrupts.
May be, overflow check/polling timeout can be made configurable (module parameter).
--
regards,
-grygorii
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] misc: eeprom: implement compatible DT probing
From: Peter Rosin @ 2016-12-08 18:23 UTC (permalink / raw)
To: Linus Walleij, Wolfram Sang
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1481219279-6982-1-git-send-email-linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
On 2016-12-08 18:47, Linus Walleij wrote:
> The compatible string for an EEPROM in the device tree is currently
> completely ignored by the kernel, simply stated it will not make the
> corresponding AT24 EEPROM driver probe properly. It is instead still
> relying on the DT node name to be set to one of the I2C device IDs
> which works due to a side effect in the I2C DT parsing code.
>
> Fix this up by making the DT probe mechanism a bit more elaborate:
> actually match on the compatible strings defined in the device
> tree bindings in Documentation/devicetree/bindings/eeprom/eeprom.txt:
> map these to the corresponding I2C IDs by name and look up the
> magic flags from the I2C ID before proceeding, also make the DT
> compatible string take precedence.
>
> Keep the second DT parsing callback that sets up per-chip flags as
> this needs to happen after mangling the magic flags passed from the
> I2C ID table.
>
> All vendor compatible strings listed in the binding document are
> added to the driver.
>
> After this it is possible to name the device tree node for the EEPROM
> whatever you actually like to call it, and the probing will be done
> from the compatible string.
>
> Before this patch, the following device tree node does not probe,
> which might be considered a bug:
>
> eeprom@52 {
> compatible = "atmel,at24c128";
The way I read it, that should be "atmel,24c128", i.e. w/o the "at" prefix.
> reg = <0x52>;
> pagesize = <64>;
> };
>
> After this patch, the driver probes fine from this node.
The bindings says:
Required properties:
- compatible should be "<manufacturer>,<type>", like these:
and then lists the compatibles you have added to the match table (but you
have this extra "at" prefix for the atmel manufacturer).
The way I read the above, you are free to use any manufacturer and still
have it work, and indeed, I have success with this node:
eeprom@50 {
compatible = "nxp,24c02";
reg = <0x50>;
pagesize = <16>;
};
I fear that your patch will regress this matching on the wildcard
manufacturer. I haven't tested that though, but there are enough
question marks anyway...
Cheers,
Peter
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 1/2] devicetree: i2c-hid: Add Wacom digitizer + regulator support
From: Dmitry Torokhov @ 2016-12-08 18:12 UTC (permalink / raw)
To: Rob Herring
Cc: Benjamin Tissoires, Doug Anderson, Brian Norris, Jiri Kosina,
Caesar Wang, open list:ARM/Rockchip SoC...,
linux-input-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Mark Rutland
In-Reply-To: <CAL_JsqKu0yhLVyEjcZs_rn=VqM9O4F_VMhOkfhEEvbYAjvWSTg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Thu, Dec 08, 2016 at 10:26:41AM -0600, Rob Herring wrote:
> On Thu, Dec 8, 2016 at 10:13 AM, Dmitry Torokhov
> <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > On December 8, 2016 8:03:06 AM PST, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> >>On Thu, Dec 8, 2016 at 9:41 AM, Benjamin Tissoires
> >><benjamin.tissoires-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> >>> On Dec 06 2016 or thereabouts, Doug Anderson wrote:
> >>>> Hi,
> >>>>
> >>>> On Tue, Dec 6, 2016 at 6:56 AM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> >>>> > On Tue, Dec 6, 2016 at 2:48 AM, Benjamin Tissoires
> >>>> > <benjamin.tissoires-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> >>>> >> On Dec 05 2016 or thereabouts, Rob Herring wrote:
> >>>> >>> On Thu, Dec 01, 2016 at 09:24:50AM -0800, Brian Norris wrote:
> >>>> >>> > Hi Benjamin and Rob,
> >>>> >>> >
> >>>> >>> > On Thu, Dec 01, 2016 at 03:34:34PM +0100, Benjamin Tissoires
> >>wrote:
> >>>> >>> > > On Nov 30 2016 or thereabouts, Brian Norris wrote:
> >>>> >>> > > > From: Caesar Wang <wxt-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> >>>> >>> > > >
> >>>> >>> > > > Add a compatible string and regulator property for Wacom
> >>W9103
> >>>> >>> > > > digitizer. Its VDD supply may need to be enabled before
> >>using it.
> >>>> >>> > > >
> >>>> >>> > > > Signed-off-by: Caesar Wang <wxt-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> >>>> >>> > > > Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> >>>> >>> > > > Cc: Jiri Kosina <jikos-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> >>>> >>> > > > Cc: linux-input-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> >>>> >>> > > > Signed-off-by: Brian Norris <briannorris-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> >>>> >>> > > > ---
> >>>> >>> > > > v1 was a few months back. I finally got around to
> >>rewriting it based on
> >>>> >>> > > > DT binding feedback.
> >>>> >>> > > >
> >>>> >>> > > > v2:
> >>>> >>> > > > * add compatible property for wacom
> >>>> >>> > > > * name the regulator property specifically (VDD)
> >>>> >>> > > >
> >>>> >>> > > > Documentation/devicetree/bindings/input/hid-over-i2c.txt
> >>| 6 +++++-
> >>>> >>> > > > 1 file changed, 5 insertions(+), 1 deletion(-)
> >>>> >>> > > >
> >>>> >>> > > > diff --git
> >>a/Documentation/devicetree/bindings/input/hid-over-i2c.txt
> >>b/Documentation/devicetree/bindings/input/hid-over-i2c.txt
> >>>> >>> > > > index 488edcb264c4..eb98054e60c9 100644
> >>>> >>> > > > ---
> >>a/Documentation/devicetree/bindings/input/hid-over-i2c.txt
> >>>> >>> > > > +++
> >>b/Documentation/devicetree/bindings/input/hid-over-i2c.txt
> >>>> >>> > > > @@ -11,12 +11,16 @@ If this binding is used, the kernel
> >>module i2c-hid will handle the communication
> >>>> >>> > > > with the device and the generic hid core layer will
> >>handle the protocol.
> >>>> >>> > > >
> >>>> >>> > > > Required properties:
> >>>> >>> > > > -- compatible: must be "hid-over-i2c"
> >>>> >>> > > > +- compatible: must be "hid-over-i2c", or a
> >>device-specific string like:
> >>>> >>> > > > + * "wacom,w9013"
> >>>> >>> > >
> >>>> >>> > > NACK on this one.
> >>>> >>> > >
> >>>> >>> > > After re-reading the v1 submission I realized Rob asked for
> >>this change,
> >>>> >>> > > but I strongly disagree.
> >>>> >>> > >
> >>>> >>> > > HID over I2C is a generic protocol, in the same way HID over
> >>USB is. We
> >>>> >>> > > can not start adding device specifics here, this is opening
> >>the can of
> >>>> >>> > > worms. If the device is a HID one, nothing else should
> >>matter. The rest
> >>>> >>> > > (description of the device, name, etc...) is all provided by
> >>the
> >>>> >>> > > protocol.
> >>>> >>> >
> >>>> >>> > I should have spoken up when Rob made the suggestion, because
> >>I more or
> >>>> >>> > less agree with Benjamin here. I don't really see why this
> >>needs to have
> >>>> >>> > a specialized compatible string, as the property is still
> >>fairly
> >>>> >>> > generic, and the entire device handling is via a generic
> >>protocol. The
> >>>> >>> > fact that we manage its power via a regulator is not very
> >>>> >>> > device-specific.
> >>>> >>>
> >>>> >>> It doesn't matter that the protocol is generic. The device
> >>attached and
> >>>> >>> the implementation is not. Implementations have been known to
> >>have
> >>>> >>> bugs/quirks (generally speaking, not HID over I2C in
> >>particular). There
> >>>> >>> are also things outside the scope of what is 'hid-over-i2c' like
> >>what's
> >>>> >>> needed to power-on the device which this patch clearly show.
> >>>> >>
> >>>> >> Yes, there are bugs, quirks, even with HID. But the HID declares
> >>within
> >>>> >> the protocol the Vendor ID and the Product ID, which means once
> >>we pass
> >>>> >> the initial "device is ready" step and can do a single i2c
> >>write/read,
> >>>> >> we don't give a crap about device tree anymore.
> >>>> >>
> >>>> >> This is just about setting the device in shape so that it can
> >>answer a
> >>>> >> single write/read.
> >>>> >>
> >>>> >>>
> >>>> >>> This is no different than a panel attached via LVDS, eDP, etc.,
> >>or
> >>>> >>> USB/PCIe device hard-wired on a board. They all use standard
> >>protocols
> >>>> >>> and all need additional data to describe them. Of course, adding
> >>a
> >>>> >>> single property for a delay would not be a big deal, but it's
> >>never
> >>>> >>> ending. Next you need multiple supplies, GPIO controls, mutiple
> >>>> >>> delays... This has been discussed to death already. As Thierry
> >>Reding
> >>>> >>> said, you're not special[1].
> >>>> >>
> >>>> >> I can somewhat understand what you mean. The official
> >>specification is
> >>>> >> for ACPI. And ACPI allows to calls various settings while
> >>querying the
> >>>> >> _STA method for instance. So in the ACPI world, we don't need to
> >>care
> >>>> >> about regulators or GPIOs because the OEM deals with this in its
> >>own
> >>>> >> blob.
> >>>> >>
> >>>> >> Now, coming back to our issue. We are not special, maybe, if he
> >>says so.
> >>>> >> But this really feels like a design choice between putting the
> >>burden on
> >>>> >> device tree and OEMs or in the module maintainers. And I'd rather
> >>have
> >>>> >> the OEM deal with their device than me having to update the
> >>module for
> >>>> >> each generations of hardware. Indeed, this looks like an
> >>"endless"
> >>>> >> amount of quirks, but I'd rather have this endless amount of
> >>quirks than
> >>>> >> having to maintain an endless amount of list of new devices that
> >>behaves
> >>>> >> the same way. We are talking here about "wacom,w9013", but then
> >>comes
> >>>> >> "wacom,w9014" and we need to upgrade the kernel.
> >>>> >
> >>>> > No. If the w9014 can claim compatibility with then w9013, then you
> >>>> > don't need a kernel change. The DT should list the w9014 AND
> >>w9013,
> >>>> > but the kernel only needs to know about the w9013. That is until
> >>there
> >>>> > is some difference which is why the DT should list w9014 to start
> >>>> > with.
> >>>> >
> >>>> > If you don't have any power control to deal with, then the kernel
> >>can
> >>>> > always just match on "hid-over-i2c" compatible.
> >>>>
> >>>> Just my $0.02. Feel free to ignore.
> >>>>
> >>>> One thought is that I would say that the need to power on the device
> >>>> explicitly seems more like a board level difference and less like a
> >>>> difference associated with a particular digitizer. Said another
> >>way,
> >>>> it seems likely there will be boards with a w9013 without explicit
> >>>> control of the regulator in software and it seems like there will be
> >>>> boards with other digitizers where suddenly a new board will come
> >>out
> >>>> that needs explicit control of the regulator.
> >>>>
> >>>> In this particular case I feel like we can draw a lot of parallels
> >>to
> >>>> an SDIO bus.
> >>>>
> >>>> When you specify an SDIO bus you don't specify what kind of card
> >>will
> >>>> be present, you just say "I've got an SDIO bus" and then the
> >>specific
> >>>> device underneath is probed. Here we've say "I've got an i2c
> >>>> connection intended for HID" and then you probe for the HID device
> >>>> that's on the connection.
> >>>>
> >>>> Also for an SDIO bus, you've possibly got a regulators / GPIOs /
> >>>> resets that need to be controlled, but the specific details of these
> >>>> regulator / GPIOs / resets are specific to a given board and not
> >>>> necessarily a given SDIO device.
> >>>>
> >>>
> >>> Thanks Doug for this. I had the feeling this wasn't right, but you
> >>> actually managed to put the words on it. If it's a board problem (if
> >>> you switch the wacom device with an other HID over I2C device and you
> >>> still need the same regulator/timing parameters), then this should
> >>> simply be mentioned on the patch.
> >>>
> >>> So Brian, could you please respin the series and remve the Wacom
> >>> mentions and explain that it is required for the board itself?
> >>
> >>In advance, NAK.
> >>
> >>This is not how DT works. Either this binding needs a Wacom compatible
> >>or don't use DT.
> >>
> >
> > And if tomorrow there is Elan device that is drop-in compatible
> > (same connector, etc) with Wacom i2c-hid, will you ask for
> > Elan-specific binding? Atmel? Weida? They all need to be powered up
> > ultimately.
>
> Yes, I will. Anyone who's worked on drop-in or pin compatible parts
> knows there's no such thing.
And yet we are shipping quite a few of Chromebooks with touchpads that
are dual-sourced and can be exchanged at any time without any changes to
the software (be it kernel or firmware).
>
> That in no way means the OS driver has to know about each and every
> one. If they can all claim compatibility with Wacom (including power
> control), then they can have a Wacom compatible string too. Or you can
> just never tell me that there's a different manufacturer and I won't
> care as long you don't need different control. But soon as a device
> needs another power rail, GPIO or different timing, then you'd better
> have a new compatible string.
That I simply do not understand. We routinely enhance bindings because
the devices get used on different boards that expose more or less
connections. I.e. quite often we start with a device whose rails are
controlled by the firmware, and so the binding only contains register
and interrupt data. Then we come across board that exposes reset GPIO
and we add that to the binding (bit we do not invent new compatible
string just because there is GPIO now). Then we get a board that
actually wants kernel to control power to the chip and we add a
regulator. Non-optional, mind you, because we rely on the regulator
system to give us a dummy one if it is not described by the
firmware/other means. And then we get another board that exposes another
power rail (let's say 3.3V to the panel whereas the previous one did not
use it). And we add another regulator binding. All this time we have
the same compatibility string.
So in this case we finally got to the point where we admit that devices
speaking HID over I2C do have power rails; we simply did not need to
control them before. The same Wacom digitizer, that you now demand to
add a compatible for, may have been used in other boards where power
rails were either turned on by the firmware at boot time and left on
until the board is shutdown, or ACPI was controlling them (via _ON/_OFF
methods), or there was some other magic. Having a supply and ability to
control the time it takes to bring the device into operating state is in
no way Wacom-specific, so why new compatibility instead of enhancing
the current binding?
And on top of that, currently multiple compatible strings are utterly
broken with regard to module loading (you only emit modalias for the
first component), so having DTS with multiples does not work well in
real life.
--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ 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