* [PATCH v2 2/9] clk: stm32f4: Add PLL_I2S & PLL_SAI for STM32F429/469 boards
From: gabriel.fernandez @ 2016-11-24 14:45 UTC (permalink / raw)
To: Rob Herring, Mark Rutland, Russell King, Maxime Coquelin,
Alexandre Torgue, Michael Turquette, Stephen Boyd, Nicolas Pitre,
Arnd Bergmann, daniel.thompson, andrea.merello, radoslaw.pietrzyk
Cc: devicetree, linux-arm-kernel, linux-kernel, linux-clk, kernel,
gabriel.fernandez, ludovic.barre, olivier.bideau, amelie.delaunay
In-Reply-To: <1479998749-20358-1-git-send-email-gabriel.fernandez@st.com>
From: Gabriel Fernandez <gabriel.fernandez@st.com>
This patch introduces PLL_I2S and PLL_SAI.
Vco clock of these PLLs can be modify by DT (only n multiplicator,
m divider is still fixed by the boot-loader).
Each PLL has 3 dividers. PLL should be off when we modify the rate.
Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
---
.../devicetree/bindings/clock/st,stm32-rcc.txt | 2 +
drivers/clk/clk-stm32f4.c | 342 ++++++++++++++++++++-
include/dt-bindings/clock/stm32f4-clock.h | 4 +-
3 files changed, 332 insertions(+), 16 deletions(-)
diff --git a/Documentation/devicetree/bindings/clock/st,stm32-rcc.txt b/Documentation/devicetree/bindings/clock/st,stm32-rcc.txt
index 18e05c2..eb6733c 100644
--- a/Documentation/devicetree/bindings/clock/st,stm32-rcc.txt
+++ b/Documentation/devicetree/bindings/clock/st,stm32-rcc.txt
@@ -74,6 +74,8 @@ The secondary index is bound with the following magic numbers:
crystal or ceramic resonator)
4 CLK_HSE_RTC (HSE division factor for RTC clock)
5 CLK_RTC (real-time clock)
+ 6 PLL_VCO_I2S (vco frequency of I2S pll)
+ 7 PLL_VCO_SAI (vco frequency of SAI pll)
Example:
diff --git a/drivers/clk/clk-stm32f4.c b/drivers/clk/clk-stm32f4.c
index 39965ab..c2b62cc 100644
--- a/drivers/clk/clk-stm32f4.c
+++ b/drivers/clk/clk-stm32f4.c
@@ -35,6 +35,7 @@
*/
#include <dt-bindings/clock/stm32f4-clock.h>
+#define STM32F4_RCC_CR 0x00
#define STM32F4_RCC_PLLCFGR 0x04
#define STM32F4_RCC_CFGR 0x08
#define STM32F4_RCC_AHB1ENR 0x30
@@ -44,6 +45,8 @@
#define STM32F4_RCC_APB2ENR 0x44
#define STM32F4_RCC_BDCR 0x70
#define STM32F4_RCC_CSR 0x74
+#define STM32F4_RCC_PLLI2SCFGR 0x84
+#define STM32F4_RCC_PLLSAICFGR 0x88
struct stm32f4_gate_data {
u8 offset;
@@ -329,23 +332,312 @@ static struct clk *clk_register_apb_mul(struct device *dev, const char *name,
return clk;
}
-/*
- * Decode current PLL state and (statically) model the state we inherit from
- * the bootloader.
- */
-static void stm32f4_rcc_register_pll(const char *hse_clk, const char *hsi_clk)
+enum {
+ PLL,
+ PLL_I2S,
+ PLL_SAI,
+};
+
+static const struct clk_div_table pll_divp_table[] = {
+ { 0, 2 }, { 1, 4 }, { 2, 6 }, { 3, 8 }, { 0 }
+};
+
+static const struct clk_div_table pll_divr_table[] = {
+ { 2, 2 }, { 3, 3 }, { 4, 4 }, { 5, 5 }, { 6, 6 }, { 7, 7 }, { 0 }
+};
+
+struct stm32f4_pll {
+ spinlock_t *lock;
+ struct clk_gate gate;
+ u8 offset;
+ u8 bit_rdy_idx;
+ u8 status;
+ u8 n_start;
+};
+
+#define to_stm32f4_pll(_gate) container_of(_gate, struct stm32f4_pll, gate)
+
+struct stm32f4_vco_data {
+ const char *vco_name;
+ u8 offset;
+ u8 bit_idx;
+ u8 bit_rdy_idx;
+};
+
+static const struct stm32f4_vco_data vco_data[] = {
+ { "vco", STM32F4_RCC_PLLCFGR, 24, 25 },
+ { "vco-i2s", STM32F4_RCC_PLLI2SCFGR, 26, 27 },
+ { "vco-sai", STM32F4_RCC_PLLSAICFGR, 28, 29 },
+};
+
+struct stm32f4_div_data {
+ u8 shift;
+ u8 width;
+ u8 flag_div;
+ const struct clk_div_table *div_table;
+};
+
+#define MAX_PLL_DIV 3
+static const struct stm32f4_div_data div_data[MAX_PLL_DIV] = {
+ { 16, 2, 0, pll_divp_table },
+ { 24, 4, CLK_DIVIDER_ONE_BASED, NULL },
+ { 28, 3, 0, pll_divr_table },
+};
+
+struct stm32f4_pll_data {
+ u8 pll_num;
+ u8 n_start;
+ const char *div_name[MAX_PLL_DIV];
+};
+
+static const struct stm32f4_pll_data stm32f429_pll[MAX_PLL_DIV] = {
+ { PLL, 192, { "pll", "pll48", NULL } },
+ { PLL_I2S, 192, { NULL, "plli2s-q", "plli2s-r" } },
+ { PLL_SAI, 49, { NULL, "pllsai-q", "pllsai-r" } },
+};
+
+static const struct stm32f4_pll_data stm32f469_pll[MAX_PLL_DIV] = {
+ { PLL, 50, { "pll", "pll-q", NULL } },
+ { PLL_I2S, 50, { "plli2s-p", "plli2s-q", "plli2s-r" } },
+ { PLL_SAI, 50, { "pllsai-p", "pllsai-q", "pllsai-r" } },
+};
+
+static int stm32f4_pll_is_enabled(struct clk_hw *hw)
+{
+ return clk_gate_ops.is_enabled(hw);
+}
+
+static int stm32f4_pll_enable(struct clk_hw *hw)
+{
+ struct clk_gate *gate = to_clk_gate(hw);
+ struct stm32f4_pll *pll = to_stm32f4_pll(gate);
+ int ret = 0;
+ unsigned long reg;
+
+ ret = clk_gate_ops.enable(hw);
+
+ ret = readl_relaxed_poll_timeout_atomic(base + STM32F4_RCC_CR, reg,
+ reg & (1 << pll->bit_rdy_idx), 0, 10000);
+
+ return ret;
+}
+
+static void stm32f4_pll_disable(struct clk_hw *hw)
{
- unsigned long pllcfgr = readl(base + STM32F4_RCC_PLLCFGR);
+ clk_gate_ops.disable(hw);
+}
- unsigned long pllm = pllcfgr & 0x3f;
- unsigned long plln = (pllcfgr >> 6) & 0x1ff;
- unsigned long pllp = BIT(((pllcfgr >> 16) & 3) + 1);
- const char *pllsrc = pllcfgr & BIT(22) ? hse_clk : hsi_clk;
- unsigned long pllq = (pllcfgr >> 24) & 0xf;
+static unsigned long stm32f4_pll_recalc(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ struct clk_gate *gate = to_clk_gate(hw);
+ struct stm32f4_pll *pll = to_stm32f4_pll(gate);
+ unsigned long n;
- clk_register_fixed_factor(NULL, "vco", pllsrc, 0, plln, pllm);
- clk_register_fixed_factor(NULL, "pll", "vco", 0, 1, pllp);
- clk_register_fixed_factor(NULL, "pll48", "vco", 0, 1, pllq);
+ n = (readl(base + pll->offset) >> 6) & 0x1ff;
+
+ return parent_rate * n;
+}
+
+static long stm32f4_pll_round_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long *prate)
+{
+ struct clk_gate *gate = to_clk_gate(hw);
+ struct stm32f4_pll *pll = to_stm32f4_pll(gate);
+ unsigned long n;
+
+ n = rate / *prate;
+
+ if (n < pll->n_start)
+ n = pll->n_start;
+ else if (n > 432)
+ n = 432;
+
+ return *prate * n;
+}
+
+static int stm32f4_pll_set_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long parent_rate)
+{
+ struct clk_gate *gate = to_clk_gate(hw);
+ struct stm32f4_pll *pll = to_stm32f4_pll(gate);
+
+ unsigned long n;
+ unsigned long val;
+ int pll_state;
+
+ pll_state = stm32f4_pll_is_enabled(hw);
+
+ if (pll_state)
+ stm32f4_pll_disable(hw);
+
+ n = rate / parent_rate;
+
+ val = readl(base + pll->offset) & ~(0x1ff << 6);
+
+ writel(val | ((n & 0x1ff) << 6), base + pll->offset);
+
+ if (pll_state)
+ stm32f4_pll_enable(hw);
+
+ return 0;
+}
+
+static const struct clk_ops stm32f4_pll_gate_ops = {
+ .enable = stm32f4_pll_enable,
+ .disable = stm32f4_pll_disable,
+ .is_enabled = stm32f4_pll_is_enabled,
+ .recalc_rate = stm32f4_pll_recalc,
+ .round_rate = stm32f4_pll_round_rate,
+ .set_rate = stm32f4_pll_set_rate,
+};
+
+struct stm32f4_pll_div {
+ struct clk_divider div;
+ struct clk_hw *hw_pll;
+};
+
+#define to_pll_div_clk(_div) container_of(_div, struct stm32f4_pll_div, div)
+
+static unsigned long stm32f4_pll_div_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ return clk_divider_ops.recalc_rate(hw, parent_rate);
+}
+
+static long stm32f4_pll_div_round_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long *prate)
+{
+ return clk_divider_ops.round_rate(hw, rate, prate);
+}
+
+static int stm32f4_pll_div_set_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long parent_rate)
+{
+ int pll_state, ret;
+
+ struct clk_divider *div = to_clk_divider(hw);
+ struct stm32f4_pll_div *pll_div = to_pll_div_clk(div);
+
+ pll_state = stm32f4_pll_is_enabled(pll_div->hw_pll);
+
+ if (pll_state)
+ stm32f4_pll_disable(pll_div->hw_pll);
+
+ ret = clk_divider_ops.set_rate(hw, rate, parent_rate);
+
+ if (pll_state)
+ stm32f4_pll_enable(pll_div->hw_pll);
+
+ return ret;
+}
+
+const struct clk_ops stm32f4_pll_div_ops = {
+ .recalc_rate = stm32f4_pll_div_recalc_rate,
+ .round_rate = stm32f4_pll_div_round_rate,
+ .set_rate = stm32f4_pll_div_set_rate,
+};
+
+static struct clk_hw *clk_register_pll_div(const char *name,
+ const char *parent_name, unsigned long flags,
+ void __iomem *reg, u8 shift, u8 width,
+ u8 clk_divider_flags, const struct clk_div_table *table,
+ struct clk_hw *pll_hw, spinlock_t *lock)
+{
+ struct stm32f4_pll_div *pll_div;
+ struct clk_hw *hw;
+ struct clk_init_data init;
+ int ret;
+
+ /* allocate the divider */
+ pll_div = kzalloc(sizeof(*pll_div), GFP_KERNEL);
+ if (!pll_div)
+ return ERR_PTR(-ENOMEM);
+
+ init.name = name;
+ init.ops = &stm32f4_pll_div_ops;
+ init.flags = flags;
+ init.parent_names = (parent_name ? &parent_name : NULL);
+ init.num_parents = (parent_name ? 1 : 0);
+
+ /* struct clk_divider assignments */
+ pll_div->div.reg = reg;
+ pll_div->div.shift = shift;
+ pll_div->div.width = width;
+ pll_div->div.flags = clk_divider_flags;
+ pll_div->div.lock = lock;
+ pll_div->div.table = table;
+ pll_div->div.hw.init = &init;
+
+ pll_div->hw_pll = pll_hw;
+
+ /* register the clock */
+ hw = &pll_div->div.hw;
+ ret = clk_hw_register(NULL, hw);
+ if (ret) {
+ kfree(pll_div);
+ hw = ERR_PTR(ret);
+ }
+
+ return hw;
+}
+
+static struct clk_hw *stm32f4_rcc_register_pll(const char *pllsrc,
+ const struct stm32f4_pll_data *data, spinlock_t *lock)
+{
+ struct stm32f4_pll *pll;
+ struct clk_init_data init = { NULL };
+ void __iomem *reg;
+ struct clk_hw *pll_hw;
+ int ret;
+ int i;
+ const struct stm32f4_vco_data *vco;
+
+
+ pll = kzalloc(sizeof(*pll), GFP_KERNEL);
+ if (!pll)
+ return ERR_PTR(-ENOMEM);
+
+ vco = &vco_data[data->pll_num];
+
+ init.name = vco->vco_name;
+ init.ops = &stm32f4_pll_gate_ops;
+ init.flags = CLK_SET_RATE_GATE;
+ init.parent_names = &pllsrc;
+ init.num_parents = 1;
+
+ pll->gate.lock = lock;
+ pll->gate.reg = base + STM32F4_RCC_CR;
+ pll->gate.bit_idx = vco->bit_idx;
+ pll->gate.hw.init = &init;
+
+ pll->offset = vco->offset;
+ pll->n_start = data->n_start;
+ pll->bit_rdy_idx = vco->bit_rdy_idx;
+ pll->status = (readl(base + STM32F4_RCC_CR) >> vco->bit_idx) & 0x1;
+
+ reg = base + pll->offset;
+
+ pll_hw = &pll->gate.hw;
+ ret = clk_hw_register(NULL, pll_hw);
+ if (ret) {
+ kfree(pll);
+ return ERR_PTR(ret);
+ }
+
+ for (i = 0; i < MAX_PLL_DIV; i++)
+ if (data->div_name[i])
+ clk_register_pll_div(data->div_name[i],
+ vco->vco_name,
+ 0,
+ reg,
+ div_data[i].shift,
+ div_data[i].width,
+ div_data[i].flag_div,
+ div_data[i].div_table,
+ pll_hw,
+ lock);
+ return pll_hw;
}
/*
@@ -620,18 +912,21 @@ struct stm32f4_clk_data {
const struct stm32f4_gate_data *gates_data;
const u64 *gates_map;
int gates_num;
+ const struct stm32f4_pll_data *pll_data;
};
static const struct stm32f4_clk_data stm32f429_clk_data = {
.gates_data = stm32f429_gates,
.gates_map = stm32f42xx_gate_map,
.gates_num = ARRAY_SIZE(stm32f429_gates),
+ .pll_data = stm32f429_pll,
};
static const struct stm32f4_clk_data stm32f469_clk_data = {
.gates_data = stm32f469_gates,
.gates_map = stm32f46xx_gate_map,
.gates_num = ARRAY_SIZE(stm32f469_gates),
+ .pll_data = stm32f469_pll,
};
static const struct of_device_id stm32f4_of_match[] = {
@@ -652,6 +947,9 @@ static void __init stm32f4_rcc_init(struct device_node *np)
int n;
const struct of_device_id *match;
const struct stm32f4_clk_data *data;
+ unsigned long pllcfgr;
+ const char *pllsrc;
+ unsigned long pllm;
base = of_iomap(np, 0);
if (!base) {
@@ -682,7 +980,21 @@ static void __init stm32f4_rcc_init(struct device_node *np)
clk_register_fixed_rate_with_accuracy(NULL, "hsi", NULL, 0,
16000000, 160000);
- stm32f4_rcc_register_pll(hse_clk, "hsi");
+ pllcfgr = readl(base + STM32F4_RCC_PLLCFGR);
+ pllsrc = pllcfgr & BIT(22) ? hse_clk : "hsi";
+ pllm = pllcfgr & 0x3f;
+
+ clk_hw_register_fixed_factor(NULL, "vco_in", pllsrc,
+ 0, 1, pllm);
+
+ stm32f4_rcc_register_pll("vco_in", &data->pll_data[0],
+ &stm32f4_clk_lock);
+
+ clks[PLL_VCO_I2S] = stm32f4_rcc_register_pll("vco_in",
+ &data->pll_data[1], &stm32f4_clk_lock);
+
+ clks[PLL_VCO_SAI] = stm32f4_rcc_register_pll("vco_in",
+ &data->pll_data[2], &stm32f4_clk_lock);
sys_parents[1] = hse_clk;
clk_register_mux_table(
diff --git a/include/dt-bindings/clock/stm32f4-clock.h b/include/dt-bindings/clock/stm32f4-clock.h
index 3132b6a..56b8e10 100644
--- a/include/dt-bindings/clock/stm32f4-clock.h
+++ b/include/dt-bindings/clock/stm32f4-clock.h
@@ -25,7 +25,9 @@
#define CLK_LSE 3
#define CLK_HSE_RTC 4
#define CLK_RTC 5
+#define PLL_VCO_I2S 6
+#define PLL_VCO_SAI 7
-#define END_PRIMARY_CLK 6
+#define END_PRIMARY_CLK 8
#endif
--
1.9.1
^ permalink raw reply related
* [PATCH v2 1/9] clk: stm32f4: Update DT bindings documentation
From: gabriel.fernandez-qxv4g6HH51o @ 2016-11-24 14:45 UTC (permalink / raw)
To: Rob Herring, Mark Rutland, Russell King, Maxime Coquelin,
Alexandre Torgue, Michael Turquette, Stephen Boyd, Nicolas Pitre,
Arnd Bergmann, daniel.thompson-QSEj5FYQhm4dnm+yROfE0A,
andrea.merello-Re5JQEeQqe8AvxtiuMwx3w,
radoslaw.pietrzyk-Re5JQEeQqe8AvxtiuMwx3w
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-clk-u79uwXL29TY76Z2rM5mHXA, kernel-F5mvAk5X5gdBDgjK7y7TUQ,
gabriel.fernandez-qxv4g6HH51o, ludovic.barre-qxv4g6HH51o,
olivier.bideau-qxv4g6HH51o, amelie.delaunay-qxv4g6HH51o
In-Reply-To: <1479998749-20358-1-git-send-email-gabriel.fernandez-qxv4g6HH51o@public.gmane.org>
From: Gabriel Fernandez <gabriel.fernandez-qxv4g6HH51o@public.gmane.org>
Creation of dt include file for specific stm32f4 clocks.
These specific clocks are not derived from system clock (SYSCLOCK)
We should use index 1 to use these clocks in DT.
e.g. <&rcc 1 CLK_LSI>
Signed-off-by: Gabriel Fernandez <gabriel.fernandez-qxv4g6HH51o@public.gmane.org>
---
.../devicetree/bindings/clock/st,stm32-rcc.txt | 8 ++++++
drivers/clk/clk-stm32f4.c | 9 +++++--
include/dt-bindings/clock/stm32f4-clock.h | 31 ++++++++++++++++++++++
3 files changed, 46 insertions(+), 2 deletions(-)
create mode 100644 include/dt-bindings/clock/stm32f4-clock.h
diff --git a/Documentation/devicetree/bindings/clock/st,stm32-rcc.txt b/Documentation/devicetree/bindings/clock/st,stm32-rcc.txt
index 0532d81..18e05c2 100644
--- a/Documentation/devicetree/bindings/clock/st,stm32-rcc.txt
+++ b/Documentation/devicetree/bindings/clock/st,stm32-rcc.txt
@@ -17,6 +17,8 @@ Required properties:
property, containing a phandle to the clock device node, an index selecting
between gated clocks and other clocks and an index specifying the clock to
use.
+- clocks: External oscillator clock phandle
+ - high speed external clock signal (HSE)
Example:
@@ -25,6 +27,7 @@ Example:
#clock-cells = <2>
compatible = "st,stm32f42xx-rcc", "st,stm32-rcc";
reg = <0x40023800 0x400>;
+ clocks = <&clk_hse>;
};
Specifying gated clocks
@@ -66,6 +69,11 @@ The secondary index is bound with the following magic numbers:
0 SYSTICK
1 FCLK
+ 2 CLK_LSI (low-power clock source)
+ 3 CLK_LSE (generated from a 32.768 kHz low-speed external
+ crystal or ceramic resonator)
+ 4 CLK_HSE_RTC (HSE division factor for RTC clock)
+ 5 CLK_RTC (real-time clock)
Example:
diff --git a/drivers/clk/clk-stm32f4.c b/drivers/clk/clk-stm32f4.c
index 5eb05db..39965ab 100644
--- a/drivers/clk/clk-stm32f4.c
+++ b/drivers/clk/clk-stm32f4.c
@@ -28,6 +28,13 @@
#include <linux/regmap.h>
#include <linux/mfd/syscon.h>
+/*
+ * Include list of clocks wich are not derived from system clock (SYSCLOCK)
+ * The index of these clocks is the secondary index of DT bindings
+ *
+ */
+#include <dt-bindings/clock/stm32f4-clock.h>
+
#define STM32F4_RCC_PLLCFGR 0x04
#define STM32F4_RCC_CFGR 0x08
#define STM32F4_RCC_AHB1ENR 0x30
@@ -208,8 +215,6 @@ struct stm32f4_gate_data {
{ STM32F4_RCC_APB2ENR, 26, "ltdc", "apb2_div" },
};
-enum { SYSTICK, FCLK, CLK_LSI, CLK_LSE, CLK_HSE_RTC, CLK_RTC, END_PRIMARY_CLK };
-
/*
* This bitmask tells us which bit offsets (0..192) on STM32F4[23]xxx
* have gate bits associated with them. Its combined hweight is 71.
diff --git a/include/dt-bindings/clock/stm32f4-clock.h b/include/dt-bindings/clock/stm32f4-clock.h
new file mode 100644
index 0000000..3132b6a
--- /dev/null
+++ b/include/dt-bindings/clock/stm32f4-clock.h
@@ -0,0 +1,31 @@
+/*
+ * stm32f4-clock.h
+ *
+ * Copyright (C) 2016 STMicroelectronics
+ * Author: Gabriel Fernandez for STMicroelectronics.
+ * License terms: GNU General Public License (GPL), version 2
+ */
+
+/*
+ * List of clocks wich are not derived from system clock (SYSCLOCK)
+ *
+ * The index of these clocks is the secondary index of DT bindings
+ * (see Documentatoin/devicetree/bindings/clock/st,stm32-rcc.txt)
+ *
+ * e.g:
+ <assigned-clocks = <&rcc 1 CLK_LSE>;
+*/
+
+#ifndef _DT_BINDINGS_CLK_STMF4_H
+#define _DT_BINDINGS_CLK_STMF4_H
+
+#define SYSTICK 0
+#define FCLK 1
+#define CLK_LSI 2
+#define CLK_LSE 3
+#define CLK_HSE_RTC 4
+#define CLK_RTC 5
+
+#define END_PRIMARY_CLK 6
+
+#endif
--
1.9.1
--
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 v2 0/9] STM32F4 missing clocks
From: gabriel.fernandez @ 2016-11-24 14:45 UTC (permalink / raw)
To: Rob Herring, Mark Rutland, Russell King, Maxime Coquelin,
Alexandre Torgue, Michael Turquette, Stephen Boyd, Nicolas Pitre,
Arnd Bergmann, daniel.thompson, andrea.merello, radoslaw.pietrzyk
Cc: devicetree, linux-arm-kernel, linux-kernel, linux-clk, kernel,
gabriel.fernandez, ludovic.barre, olivier.bideau, amelie.delaunay
From: Gabriel Fernandez <gabriel.fernandez@st.com>
v2:
- Put post divider in config structure
- Rework patch-set
- add update dt binding documentation
- add clock definition file
- Use composite for pll vco clocks
- For auxiliary clock, allow the possiblity to enable peripheral
clocks at same time (sugested by radek)
- Add vco_in clock (entry frequency for all pll) to simplify the code and clarify clock tree
- Fix missing end of divider tables
This patch-set adds:
- I2S & SAI PLLs
- SDIO & 48 Mhz clocks
- LCD-TFT clock
- I2S & SAI clocks
Gabriel Fernandez (9):
clk: stm32f4: Update DT bindings documentation
clk: stm32f4: Add PLL_I2S & PLL_SAI for STM32F429/469 boards
clk: stm32f4: Add post divisor for I2S & SAI PLLs
clk: stm32f4: Add lcd-tft clock
clk: stm32f4: Add I2S clock
clk: stm32f4: Add SAI clocks
clk: stm32f4: SDIO & 48Mhz clock management for STM32F469 board
arm: dts: stm32f4: Add external I2S clock
arm: dts: stm32f4: Include auxiliary stm32f4 clock definition
.../devicetree/bindings/clock/st,stm32-rcc.txt | 15 +
arch/arm/boot/dts/stm32f429.dtsi | 9 +-
drivers/clk/clk-stm32f4.c | 585 ++++++++++++++++++++-
include/dt-bindings/clock/stm32f4-clock.h | 37 ++
4 files changed, 626 insertions(+), 20 deletions(-)
create mode 100644 include/dt-bindings/clock/stm32f4-clock.h
--
1.9.1
^ permalink raw reply
* Re: [PATCH] mfd: cpcap: Add minimal support
From: Tony Lindgren @ 2016-11-24 14:43 UTC (permalink / raw)
To: Lee Jones
Cc: Samuel Ortiz, linux-kernel, linux-omap, devicetree, Marcel Partap,
Mark Rutland, Michael Scott, Rob Herring
In-Reply-To: <20161124085926.GV10134@dell.home>
* Lee Jones <lee.jones@linaro.org> [161124 00:56]:
> On Wed, 23 Nov 2016, Tony Lindgren wrote:
>
> > * Lee Jones <lee.jones@linaro.org> [161121 03:43]:
> > > On Fri, 18 Nov 2016, Tony Lindgren wrote:
> > > > --- a/drivers/mfd/Makefile
> > > > +++ b/drivers/mfd/Makefile
> > > > @@ -97,6 +97,7 @@ obj-$(CONFIG_MFD_MC13XXX_I2C) += mc13xxx-i2c.o
> > > > obj-$(CONFIG_MFD_CORE) += mfd-core.o
> > > >
> > > > obj-$(CONFIG_EZX_PCAP) += ezx-pcap.o
> > > > +obj-$(CONFIG_MFD_CPCAP) += cpcap.o
> > >
> > > Who is the manufacturer?
> >
> > Hmm that I don't know. There seems to be both ST and TI versions
> > of this chip manufactured for Motorola. So my guess is that it
> > should be Motorola unless there's some similar catalog part
> > available from ST used by others. If anybody has more info
> > on this please let me know :)
>
> If this IP is shared amongst vendors, it usually means it was designed
> by someone else? Synopsis perhaps?
After searching around, many specs say "ST Ericsson CPCAP". So let's
assume the manufacturer should be ste.
> > > > + cpcap->vendor = (val >> 6) & 0x0007;
> > > > + cpcap->revision = ((val >> 3) & 0x0007) | ((val << 3) & 0x0038);
> > >
> > > Lots of magic numbers here. I suggest you define them.
> >
> > I'll check if some earlier code has these defined. Otherwise I'll
> > just add a comment on the lack of available documentation.
>
> *sad face*
>
> Does that mean you don't even know what they're for?
Luckily the Motorola driver folks documented all the registers.
Unfortunately the register bits just have names. I need to
check if we have names for these bits.
> > > > + error = cpcap_init_irq_bank(cpcap, 0, 0, 16);
> > >
> > > 'ret' is more traditional.
> >
> > FYI error seems to be preferred over ret as it's meaning is
> > clear, git grep "error =" drivers/input for example.
> > I can of course change it if you prefer ret over error.
>
> I'd prefer to stick to the conventions of *this* subsystem.
>
> ... and the most common convention used kernel wide:
>
> $ git grep "ret =" | wc -l
> 117976
> $ git grep "err =" | wc -l
> 56708
> $ git grep "error =" | wc -l
> 14427
OK sure will rename.
> > > > +#define CPCAP_REG_LDEB 0x1270 /* LMR Debounce Settings */
> > > > +#define CPCAP_REG_LGDET 0x1274 /* LMR GCAI Detach Detect */
> > > > +#define CPCAP_REG_LMISC 0x1278 /* LMR Misc Bits */
> > > > +#define CPCAP_REG_LMACE 0x127c /* LMR Mace IC Support */
> > > > +
> > > > +#define CPCAP_REG_TEST 0x7c00 /* Test */
> > > > +
> > > > +#define CPCAP_REG_ST_TEST1 0x7d08 /* ST Test1 */
> > > > +
> > > > +#define CPCAP_REG_ST_TEST2 0x7d18 /* ST Test2 */
> > >
> > > It would be nice to line up the entire file. #OCD
> >
> > Hmm care to clarify what you mean here? I think it's lined up with
>
> I'm missing context now you've <snip>ed.
>
> These look straight, however is the whole file lined up (as much as
> *practically* possible)?
Yeah it should be, I'll check.
> > tabs to line up. I left empty lines where the registers are not
> > contiguous. What does #OCD mean, Obsessive Compulsive Disorder over
> > header files maybe? :)
>
> Yes, that's what it means.
>
> /me likes straight lines. :)
Sure nothing wrong with that ;)
Regards,
Tony
^ permalink raw reply
* Re: [RFC PATCH net v2 0/3] Fix OdroidC2 Gigabit Tx link issue
From: Martin Blumenstingl @ 2016-11-24 14:40 UTC (permalink / raw)
To: Jerome Brunet
Cc: netdev, devicetree, Florian Fainelli, Carlo Caione, Kevin Hilman,
Giuseppe Cavallaro, Alexandre TORGUE, Andre Roth, Neil Armstrong,
linux-amlogic, linux-arm-kernel, linux-kernel
In-Reply-To: <1479742524-30222-1-git-send-email-jbrunet@baylibre.com>
Hi Jerome,
On Mon, Nov 21, 2016 at 4:35 PM, Jerome Brunet <jbrunet@baylibre.com> wrote:
> This patchset fixes an issue with the OdroidC2 board (DWMAC + RTL8211F).
> Initially reported as a low Tx throughput issue at gigabit speed, the
> platform enters LPI too often. This eventually break the link (both Tx
> and Rx), and require to bring the interface down and up again to get the
> Rx path working again.
>
> The root cause of this issue is not fully understood yet but disabling EEE
> advertisement on the PHY prevent this feature to be negotiated.
> With this change, the link is stable and reliable, with the expected
> throughput performance.
I have just sent a series which allows configuring the TX delay on the
MAC (dwmac-meson8b glue) side: [0]
Disabling the TX delay generated by the MAC fixes TX throughput for
me, even when leaving EEE enabled in the RTL8211F PHY driver!
Unfortunately the RTL8211F PHY is a black-box for the community
because there is no public datasheeet available.
*maybe* (pure speculation!) they're enabling the TX delay based on
some internal magic only when EEE is enabled.
Jerome, could you please re-test the behavior on your Odroid-C2 when
you have EEE still enabled but the TX-delay disabled?
In my case throughput is fine, and "$ ethtool -S eth0 | grep lpi" gives:
irq_tx_path_in_lpi_mode_n: 0
irq_tx_path_exit_lpi_mode_n: 0
irq_rx_path_in_lpi_mode_n: 0
irq_rx_path_exit_lpi_mode_n: 0
Regards,
Martin
[0] http://lists.infradead.org/pipermail/linux-amlogic/2016-November/001674.html
^ permalink raw reply
* [PATCH] ARM: dts: da850-lcdk: Add ethernet0 alias to DT
From: Fabien Parent @ 2016-11-24 14:35 UTC (permalink / raw)
To: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: khilman-rdvid1DuHRBWk0Htik3J/w, nsekhar-l0cyMroinI0,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
Fabien Parent
In order to avoid Linux generating a random mac address on every boot,
add an ethernet0 alias that will allow u-boot to patch the dtb with
the MAC address programmed into the EEPROM.
Signed-off-by: Fabien Parent <fparent-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
arch/arm/boot/dts/da850-lcdk.dts | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/da850-lcdk.dts b/arch/arm/boot/dts/da850-lcdk.dts
index 7b8ab21..18dfec93 100644
--- a/arch/arm/boot/dts/da850-lcdk.dts
+++ b/arch/arm/boot/dts/da850-lcdk.dts
@@ -13,6 +13,7 @@
aliases {
serial2 = &serial2;
+ ethernet0 = ð0;
};
chosen {
--
2.10.2
--
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
* [net-next PATCH v1 2/2] net: stmmac: dwmac-meson8b: make the RGMII TX delay configurable
From: Martin Blumenstingl @ 2016-11-24 14:34 UTC (permalink / raw)
To: linux-amlogic, devicetree, netdev, davem, khilman, mark.rutland,
robh+dt
Cc: linux-arm-kernel, alexandre.torgue, peppe.cavallaro, carlo,
jbrunet, Martin Blumenstingl
In-Reply-To: <20161124143417.10178-1-martin.blumenstingl@googlemail.com>
Prior to this patch we were using a hardcoded RGMII TX clock delay of
1/4 cycle (= 2ns). This value works for many boards, but unfortunately
not for all (due to the way the actual circuit is designed, sometimes
because the TX delay is enabled in the PHY, etc.).
Making the TX delay on the MAC side configurable allows us to support
all possible hardware combinations (which may or not be out there).
This allows fixing a compatibility issue on some boards, where the
RTL8211F PHY is configured to generate the TX delay. We can now turn
off the TX delay in the MAC, because otherwise we would be applying the
delay twice (which results in non-working TX traffic).
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index 250e4ce..1697d1a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -23,6 +23,8 @@
#include <linux/platform_device.h>
#include <linux/stmmac.h>
+#include <dt-bindings/net/dwmac-meson8b.h>
+
#include "stmmac_platform.h"
#define PRG_ETH0 0x0
@@ -35,10 +37,6 @@
#define PRG_ETH0_TXDLY_SHIFT 5
#define PRG_ETH0_TXDLY_MASK GENMASK(6, 5)
-#define PRG_ETH0_TXDLY_OFF (0x0 << PRG_ETH0_TXDLY_SHIFT)
-#define PRG_ETH0_TXDLY_QUARTER (0x1 << PRG_ETH0_TXDLY_SHIFT)
-#define PRG_ETH0_TXDLY_HALF (0x2 << PRG_ETH0_TXDLY_SHIFT)
-#define PRG_ETH0_TXDLY_THREE_QUARTERS (0x3 << PRG_ETH0_TXDLY_SHIFT)
/* divider for the result of m250_sel */
#define PRG_ETH0_CLK_M250_DIV_SHIFT 7
@@ -69,6 +67,8 @@ struct meson8b_dwmac {
struct clk_divider m25_div;
struct clk *m25_div_clk;
+
+ u32 tx_dly;
};
static void meson8b_dwmac_mask_bits(struct meson8b_dwmac *dwmac, u32 reg,
@@ -198,7 +198,7 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
/* TX clock delay - all known boards use a 1/4 cycle delay */
meson8b_dwmac_mask_bits(dwmac, PRG_ETH0, PRG_ETH0_TXDLY_MASK,
- PRG_ETH0_TXDLY_QUARTER);
+ dwmac->tx_dly << PRG_ETH0_TXDLY_SHIFT);
break;
case PHY_INTERFACE_MODE_RMII:
@@ -279,6 +279,12 @@ static int meson8b_dwmac_probe(struct platform_device *pdev)
return -EINVAL;
}
+ ret = of_property_read_u32(pdev->dev.of_node, "amlogic,tx-delay",
+ &dwmac->tx_dly);
+ if (ret)
+ /* default to 1/4 cycle (= 2ns for RGMII) */
+ dwmac->tx_dly = DWMAC_MESON8B_TXDLY_QUARTER_CYCLE;
+
ret = meson8b_init_clk(dwmac);
if (ret)
return ret;
--
2.10.2
^ permalink raw reply related
* [net-next PATCH v1 1/2] net: dt-bindings: add RGMII TX delay configuration to meson8b-dwmac
From: Martin Blumenstingl @ 2016-11-24 14:34 UTC (permalink / raw)
To: linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
davem-fT/PcQaiUtIeIZ0/mPfg9Q, khilman-rdvid1DuHRBWk0Htik3J/w,
mark.rutland-5wv7dgnIgG8, robh+dt-DgEjT+Ai2ygdnm+yROfE0A
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
alexandre.torgue-qxv4g6HH51o, peppe.cavallaro-qxv4g6HH51o,
carlo-KA+7E9HrN00dnm+yROfE0A, jbrunet-rdvid1DuHRBWk0Htik3J/w,
Martin Blumenstingl
In-Reply-To: <20161124143417.10178-1-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
This allows configuring the RGMII TX clock delay. This clock is
generated by the Meson 8b / GXBB DWMAC glue. The configuration depends
on the actual hardware (no delay may be needed due to the design of the
actual circuit, the PHY might add this delay, etc.).
The configuration values are provided as preprocessor macros to make the
devicetree files easier to read.
Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
Documentation/devicetree/bindings/net/meson-dwmac.txt | 11 +++++++++++
include/dt-bindings/net/dwmac-meson8b.h | 18 ++++++++++++++++++
2 files changed, 29 insertions(+)
create mode 100644 include/dt-bindings/net/dwmac-meson8b.h
diff --git a/Documentation/devicetree/bindings/net/meson-dwmac.txt b/Documentation/devicetree/bindings/net/meson-dwmac.txt
index 89e62dd..fe526d0 100644
--- a/Documentation/devicetree/bindings/net/meson-dwmac.txt
+++ b/Documentation/devicetree/bindings/net/meson-dwmac.txt
@@ -25,6 +25,17 @@ Required properties on Meson8b and newer:
- "clkin0" - first parent clock of the internal mux
- "clkin1" - second parent clock of the internal mux
+Optional properties on Meson8b and newer:
+- amlogic,tx-delay: The internal RGMII TX clock delay configuration.
+ Defaults to DWMAC_MESON8B_TXDLY_QUARTER_CYCLE
+ when not given. All possible values are defined
+ as preprocessor macro in
+ <dt-bindings/net/dwmac-meson8b.h>.
+ The delay is specified as divider for the
+ internal clock (RGMII typically uses a 125MHz
+ clock clock (= 8ns per cycle), so setting
+ DWMAC_MESON8B_TXDLY_QUARTER_CYCLE
+ results in a TX delay of 8ns/4 = 2ns.
Example for Meson6:
diff --git a/include/dt-bindings/net/dwmac-meson8b.h b/include/dt-bindings/net/dwmac-meson8b.h
new file mode 100644
index 0000000..4fc149e
--- /dev/null
+++ b/include/dt-bindings/net/dwmac-meson8b.h
@@ -0,0 +1,18 @@
+/*
+ * Devicetree constants for the Amlogic Meson8b and GXBB DWMAC glue layer
+ *
+ * Copyright (C) 2016 Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
+ *
+ * This program 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+/* TX delay configuration */
+#define DWMAC_MESON8B_TXDLY_OFF 0x0
+#define DWMAC_MESON8B_TXDLY_QUARTER_CYCLE 0x1
+#define DWMAC_MESON8B_TXDLY_HALF_CYCLE 0x2
+#define DWMAC_MESON8B_TXDLY_THREE_QUARTER_CYCLE 0x3
--
2.10.2
--
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
* [net-next PATCH v1 0/2] stmmac: dwmac-meson8b: configurable RGMII TX delay
From: Martin Blumenstingl @ 2016-11-24 14:34 UTC (permalink / raw)
To: linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
davem-fT/PcQaiUtIeIZ0/mPfg9Q, khilman-rdvid1DuHRBWk0Htik3J/w,
mark.rutland-5wv7dgnIgG8, robh+dt-DgEjT+Ai2ygdnm+yROfE0A
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
alexandre.torgue-qxv4g6HH51o, peppe.cavallaro-qxv4g6HH51o,
carlo-KA+7E9HrN00dnm+yROfE0A, jbrunet-rdvid1DuHRBWk0Htik3J/w,
Martin Blumenstingl
Currently the dwmac-meson8b stmmac glue driver uses a hardcoded 1/4
cycle TX clock delay. This seems to work fine for many boards (for
example Odroid-C2 or Amlogic's reference boards) but there are some
others where TX traffic is simply broken.
There are probably multiple reasons why it's working on some boards
while it's broken on others:
- some of Amlogic's reference boards are using a Micrel PHY
- hardware circuit design
- maybe more...
This raises a question though:
Which device is supposed to enable the TX delay when both MAC and PHY
support it? And should we implement it for each PHY / MAC separately
or should we think about a more generic solution (currently it's not
possible to disable the TX delay generated by the RTL8211F PHY via
devicetree when using phy-mode "rgmii")?
iperf3 results on my Mecool BB2 board (Meson GXM, RTL8211F PHY) with
TX clock delay disabled on the MAC (as it's enabled in the PHY driver).
TX throughput was virtually zero before:
$ iperf3 -c 192.168.1.100 -R
Connecting to host 192.168.1.100, port 5201
Reverse mode, remote host 192.168.1.100 is sending
[ 4] local 192.168.1.206 port 52828 connected to 192.168.1.100 port 5201
[ ID] Interval Transfer Bandwidth
[ 4] 0.00-1.00 sec 108 MBytes 901 Mbits/sec
[ 4] 1.00-2.00 sec 94.2 MBytes 791 Mbits/sec
[ 4] 2.00-3.00 sec 96.5 MBytes 810 Mbits/sec
[ 4] 3.00-4.00 sec 96.2 MBytes 808 Mbits/sec
[ 4] 4.00-5.00 sec 96.6 MBytes 810 Mbits/sec
[ 4] 5.00-6.00 sec 96.5 MBytes 810 Mbits/sec
[ 4] 6.00-7.00 sec 96.6 MBytes 810 Mbits/sec
[ 4] 7.00-8.00 sec 96.5 MBytes 809 Mbits/sec
[ 4] 8.00-9.00 sec 105 MBytes 884 Mbits/sec
[ 4] 9.00-10.00 sec 111 MBytes 934 Mbits/sec
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval Transfer Bandwidth Retr
[ 4] 0.00-10.00 sec 1000 MBytes 839 Mbits/sec 0 sender
[ 4] 0.00-10.00 sec 998 MBytes 837 Mbits/sec receiver
iperf Done.
$ iperf3 -c 192.168.1.100
Connecting to host 192.168.1.100, port 5201
[ 4] local 192.168.1.206 port 52832 connected to 192.168.1.100 port 5201
[ ID] Interval Transfer Bandwidth Retr Cwnd
[ 4] 0.00-1.01 sec 99.5 MBytes 829 Mbits/sec 117 139 KBytes
[ 4] 1.01-2.00 sec 105 MBytes 884 Mbits/sec 129 70.7 KBytes
[ 4] 2.00-3.01 sec 107 MBytes 889 Mbits/sec 106 187 KBytes
[ 4] 3.01-4.01 sec 105 MBytes 878 Mbits/sec 92 143 KBytes
[ 4] 4.01-5.00 sec 105 MBytes 882 Mbits/sec 140 129 KBytes
[ 4] 5.00-6.01 sec 106 MBytes 883 Mbits/sec 115 195 KBytes
[ 4] 6.01-7.00 sec 102 MBytes 863 Mbits/sec 133 70.7 KBytes
[ 4] 7.00-8.01 sec 106 MBytes 884 Mbits/sec 143 97.6 KBytes
[ 4] 8.01-9.01 sec 104 MBytes 875 Mbits/sec 124 107 KBytes
[ 4] 9.01-10.01 sec 105 MBytes 876 Mbits/sec 90 139 KBytes
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval Transfer Bandwidth Retr
[ 4] 0.00-10.01 sec 1.02 GBytes 874 Mbits/sec 1189 sender
[ 4] 0.00-10.01 sec 1.02 GBytes 873 Mbits/sec receiver
iperf Done.
Martin Blumenstingl (2):
net: dt-bindings: add RGMII TX delay configuration to meson8b-dwmac
net: stmmac: dwmac-meson8b: make the RGMII TX delay configurable
Documentation/devicetree/bindings/net/meson-dwmac.txt | 11 +++++++++++
drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 16 +++++++++++-----
include/dt-bindings/net/dwmac-meson8b.h | 18 ++++++++++++++++++
3 files changed, 40 insertions(+), 5 deletions(-)
create mode 100644 include/dt-bindings/net/dwmac-meson8b.h
--
2.10.2
--
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 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell Xenon SDHC
From: Ulf Hansson @ 2016-11-24 14:33 UTC (permalink / raw)
To: Ziji Hu
Cc: Gregory CLEMENT, Adrian Hunter,
linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Jason Cooper,
Andrew Lunn, Sebastian Hesselbarth, Rob Herring,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Thomas Petazzoni,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Jimmy Xu, Jisheng Zhang, Nadav Haklai, Ryan Gao, Doug Jones,
Victor Gu, Wei(SOCP) Liu, Wilson Ding
In-Reply-To: <3cd05a26-d340-476e-bab1-8be9d5446f47-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
[...]
>>
>>>
>>> +
>>> +static int __xenon_emmc_delay_adj_test(struct mmc_card *card)
>>> +{
>>> + int err;
>>> + u8 *ext_csd = NULL;
>>> +
>>> + err = mmc_get_ext_csd(card, &ext_csd);
>>> + kfree(ext_csd);
>>
>> Why do you read the ext csd here?
>>
> I would like to simply introduce the PHY setting of our SDHC.
> The target of the PHY setting is to achieve a perfect sampling
> point for transfers, during card initialization.
Okay, so the phy is involved when running the tuning sequence.
>
> For HS200/HS400/SDR104 whose SDCLK is more than 50MHz, SDHC HW
> will search for this sampling point with DLL's help.
Apologize for my ignorance, but what is a "DLL" in this case?
>
> For other speed mode whose SDLCK is less than or equals to 50MHz,
> SW has to scan the PHY delay line to find out this perfect sampling
> point. Our driver sends a command to verify a sampling point
> in current environment.
Ahh, okay! I guess the important part here is to not only send a
command, but also to make sure data becomes transferred on the DAT
lines, as to confirm your tuning sequence!?
In cases of HS200/HS400/SDR104 you should be able to use the
mmc_send_tuning() API, don't you think?
For the other cases (lower speed modes) which cards doesn't support
the tuning command, perhaps you can just assume the PHY scan succeeded
and then allow to core to continue with the card initialization
sequence? Or do you foresee any issues with that? My point is that, if
it will fail - it will fail anyway.
>
> As result, our SDHC driver has to implement the functionality to
> send commands and check the results, in host layer.
> If directly calling mmc_wait_for_cmd() is improper, could you please
> give us some suggestions?
>
> For eMMC, CMD8 is used to test current sampling point set in PHY.
Try to use mmc_send_tuning().
>
>>> +
>>> + return err;
>>> +}
>>> +
>>> +static int __xenon_sdio_delay_adj_test(struct mmc_card *card)
>>> +{
>>> + struct mmc_command cmd = {0};
>>> + int err;
>>> +
>>> + cmd.opcode = SD_IO_RW_DIRECT;
>>> + cmd.flags = MMC_RSP_R5 | MMC_CMD_AC;
>>> +
>>> + err = mmc_wait_for_cmd(card->host, &cmd, 0);
>>> + if (err)
>>> + return err;
>>> +
>>> + if (cmd.resp[0] & R5_ERROR)
>>> + return -EIO;
>>> + if (cmd.resp[0] & R5_FUNCTION_NUMBER)
>>> + return -EINVAL;
>>> + if (cmd.resp[0] & R5_OUT_OF_RANGE)
>>> + return -ERANGE;
>>> + return 0;
>>
>> No thanks! MMC/SD/SDIO protocol code belongs in the core.
>>
> For SDIO, SD_IO_RW_DIRECT command is sent to test current sampling point
> in PHY.
> Please help provide some suggestion to implement the command transfer.
Again, I think mmc_send_tuning() should be possible for you to use.
[...]
>>> + if (mmc->card)
>>> + card = mmc->card;
>>> + else
>>> + /*
>>> + * Only valid during initialization
>>> + * before mmc->card is set
>>> + */
>>> + card = priv->card_candidate;
>>> + if (unlikely(!card)) {
>>> + dev_warn(mmc_dev(mmc), "card is not present\n");
>>> + return -EINVAL;
>>> + }
>>
>> That your host need to hold a copy of the card pointer, tells me that
>> something is not really correct.
>>
>> I might be wrong, if this turns out to be a special case, but I doubt
>> it. Although, if it *is* a special such case, we shall most likely try
>> to extend the the mmc core layer instead of adding all these hacks in
>> your host driver.
>>
> This card pointer copies the temporary structure mmc_card
> used in mmc_init_card(), mmc_sd_init_card() and mmc_sdio_init_card().
> Since we call mmc_wait_for_cmd() to send test commands, we need a copy
> of that temporary mmc_card here in our host driver.
I see, thanks for clarifying.
>
> During PHY setting in card initialization, mmc_host->card is not updated
> yet with that temporary mmc_card. Thus we are not able to directly use
> mmc_host->card. Instead, this card pointer is introduced to enable
> mmc_wait_for_cmd().
>
> If we can improve our host driver to send test commands without mmc_card,
> this card pointer can be removed.
> Could you please share your opinion please?
The mmc_send_tuning() API takes the mmc_host as parameter. If you
convert to that, perhaps you would be able to remove the need to hold
the card pointer.
BTW, the reason why mmc_send_tuning() doesn't take the card as a
parameter, is exactly those you just described above.
[...]
Kind regards
Uffe
--
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 0/3] arm64: dts: r8a7796: Add CAN/CAN FD support
From: Simon Horman @ 2016-11-24 14:32 UTC (permalink / raw)
To: Chris Paterson
Cc: Marc Kleine-Budde, Wolfgang Grandegger, Magnus Damm, Rob Herring,
Mark Rutland, Ramesh Shanmugasundaram,
linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
linux-can-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <HK2PR0601MB1329C747CA5C6B5222C02C3FB7B60-5BHi1SMfQIfsvBovKiDY8NK/flDYrvD0nBOFsp37pqbUKgpGm//BTAC/G2K4zDHf@public.gmane.org>
Hi Chris,
On Thu, Nov 24, 2016 at 02:25:06PM +0000, Chris Paterson wrote:
> Hello Simon,
>
> From: Simon Horman [mailto:horms-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org]
> Sent: 24 November 2016 10:18
> > Hi Chris,
> >
> > On Thu, Nov 24, 2016 at 10:05:08AM +0000, Chris Paterson wrote:
> > > Hello Simon,
> > >
> > > From: Simon Horman [mailto:horms-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org]
> > > Sent: 23 November 2016 14:30
> > > > On Wed, Nov 23, 2016 at 02:18:13PM +0100, Marc Kleine-Budde wrote:
> > > > > On 11/23/2016 01:14 PM, Chris Paterson wrote:
...
> > > > Regarding the arch/arm64/boot/dts/renesas/ portion, I would like
> > > > some consideration given to what effect enabling memory above 4Gb
> > > > (64bit
> > > > addressing) would have.
> > >
> > > Can you give me some guidance here? I'm not sure what you're referring
> > > to. As far as I know the DT reg definition here is 64-bit, or are you
> > > referring to DMA usage? If the later, neither CAN driver uses DMA.
> >
> > Sorry for not being clearer.
> >
> > What I would like to know is if there are any problems in the CAN driver or
> > hardware that would prevent it from functioning with memory that requires
> > 64bit addressing present.
> >
> > If the CAN hardware cannot use DMA then DMA doesn't need to be taken
> > into account. But if it DMA could be enabled in future for CAN, for example
> > after some driver enhancements, then it would be good to know if 64bit
> > memory can be supported - if not it would imply DMA cannot be enabled.
>
> Thank you for the clarification.
>
> The CAN interface for r8a7795/6 does not support DMA.
>
> With CAN FD there is currently a H/W issue that means DMA is unusable.
> Potentially this issue could be fixed in the future and DMA support could
> be added to the driver. If this happens I can see no reason why the CAN
> FD IP wouldn't be able to handle DMA transfers when using 64bit
> addressing.
>
> >
> > As for non-DMA mode, will this function if memory above 4G is present?
> > If not then in theory such memory couldn't be enabled if the CAN driver
> > is enabled. This is my main concern.
>
> Yes, it functions fine. We have already tested this with the Renesas
> v3.3.2 BSP with >4Gb memory.
>
> If this is explanation okay for you, I'll proceed with a v2 splitting off
> the DT bindings documentation.
Thanks for the explanation. I think it would be good to proceed with a v2.
--
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] ARM: dts: AM571x-IDK Initial Support
From: Tony Lindgren @ 2016-11-24 14:29 UTC (permalink / raw)
To: Rob Herring
Cc: Lokesh Vutla, Linux OMAP Mailing List, Tero Kristo, Sekhar Nori,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Linux ARM Mailing List, spatton-l0cyMroinI0, Dave Gerlach,
Nishanth Menon
In-Reply-To: <CAL_JsqLv2t0qzJ1V6NGPj3iy9tOmMcnr3nsHxojb9aJxp9+8BQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
* Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [161122 07:43]:
> On Mon, Nov 21, 2016 at 10:17 PM, Lokesh Vutla <lokeshvutla-l0cyMroinI0@public.gmane.org> wrote:
> > From: Schuyler Patton <spatton-l0cyMroinI0@public.gmane.org>
> >
> > The AM571x-IDK board is a board based on TI's AM5718 SOC
> > which has a single core 1.5GHz A15 processor. This board is a
> > development platform for the Industrial market with:
> > - 1GB of DDR3L
> > - Dual 1Gbps Ethernet
> > - HDMI,
> > - PRU-ICSS
> > - uSD
> > - 16GB eMMC
> > - CAN
> > - RS-485
> > - PCIe
> > - USB3.0
> > - Video Input Port
> > - Industrial IO port and expansion connector
> >
> > The link to the data sheet and TRM can be found here:
> >
> > http://www.ti.com/product/AM5718
> >
> > Initial support is only for basic peripherals.
> >
> > Signed-off-by: Schuyler Patton <spatton-l0cyMroinI0@public.gmane.org>
> > Signed-off-by: Nishanth Menon <nm-l0cyMroinI0@public.gmane.org>
> > Signed-off-by: Dave Gerlach <d-gerlach-l0cyMroinI0@public.gmane.org>
> > Signed-off-by: Lokesh Vutla <lokeshvutla-l0cyMroinI0@public.gmane.org>
> > ---
> > Cahnges since v1:
> > - Dropped "ti,dra722", and "ti,dra72" from compatibles
> > - Fixes few node names as suggested by Rob.
> > Logs: http://pastebin.ubuntu.com/23515001/
>
> Please add acks when posting new versions.
I applied this with your ack as I was aware of the
context in this case :)
Tony
--
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 0/3] arm64: dts: r8a7796: Add CAN/CAN FD support
From: Chris Paterson @ 2016-11-24 14:25 UTC (permalink / raw)
To: Simon Horman
Cc: Marc Kleine-Budde, Wolfgang Grandegger, Magnus Damm, Rob Herring,
Mark Rutland, Ramesh Shanmugasundaram,
linux-renesas-soc@vger.kernel.org, devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, linux-can@vger.kernel.org
In-Reply-To: <20161124101733.GA18027@verge.net.au>
Hello Simon,
From: Simon Horman [mailto:horms@verge.net.au]
Sent: 24 November 2016 10:18
> Hi Chris,
>
> On Thu, Nov 24, 2016 at 10:05:08AM +0000, Chris Paterson wrote:
> > Hello Simon,
> >
> > From: Simon Horman [mailto:horms@verge.net.au]
> > Sent: 23 November 2016 14:30
> > > On Wed, Nov 23, 2016 at 02:18:13PM +0100, Marc Kleine-Budde wrote:
> > > > On 11/23/2016 01:14 PM, Chris Paterson wrote:
> > > > > This patch series adds CAN and CAN FD support to the r8a7796.
> > > > >
> > > > > Based on renesas-devel-20161122-v4.9-rc6.
> > > > >
> > > > > Chris Paterson (3):
> > > > > arm64: dts: r8a7796: Add CAN external clock support
> > > > > arm64: dts: r8a7796: Add CAN support
> > > > > arm64: dts: r8a7796: Add CAN FD support
> > > > >
> > > > > .../devicetree/bindings/net/can/rcar_can.txt | 12 +++--
> > > > > .../devicetree/bindings/net/can/rcar_canfd.txt | 12 +++--
> > > > > arch/arm64/boot/dts/renesas/r8a7796.dtsi | 61
> > > ++++++++++++++++++++++
> > > > > 3 files changed, 75 insertions(+), 10 deletions(-)
> > > >
> > > > For all three:
> > > >
> > > > Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>
> > > >
> > > > Who takes this series?
> > >
> > > I would like to see these patches split up so that the
> > > .../devicetree/bindings/ portions can go through you whole the
> > > arch/arm64/boot/dts/renesas/ portions go thorugh my renesas tree.
> >
> > Okay, will do.
>
> Thanks.
>
> > > Regarding the arch/arm64/boot/dts/renesas/ portion, I would like
> > > some consideration given to what effect enabling memory above 4Gb
> > > (64bit
> > > addressing) would have.
> >
> > Can you give me some guidance here? I'm not sure what you're referring
> > to. As far as I know the DT reg definition here is 64-bit, or are you
> > referring to DMA usage? If the later, neither CAN driver uses DMA.
>
> Sorry for not being clearer.
>
> What I would like to know is if there are any problems in the CAN driver or
> hardware that would prevent it from functioning with memory that requires
> 64bit addressing present.
>
> If the CAN hardware cannot use DMA then DMA doesn't need to be taken
> into account. But if it DMA could be enabled in future for CAN, for example
> after some driver enhancements, then it would be good to know if 64bit
> memory can be supported - if not it would imply DMA cannot be enabled.
Thank you for the clarification.
The CAN interface for r8a7795/6 does not support DMA.
With CAN FD there is currently a H/W issue that means DMA is unusable. Potentially this issue could be fixed in the future and DMA support could be added to the driver. If this happens I can see no reason why the CAN FD IP wouldn't be able to handle DMA transfers when using 64bit addressing.
>
> As for non-DMA mode, will this function if memory above 4G is present?
> If not then in theory such memory couldn't be enabled if the CAN driver is
> enabled. This is my main concern.
Yes, it functions fine. We have already tested this with the Renesas v3.3.2 BSP with >4Gb memory.
If this is explanation okay for you, I'll proceed with a v2 splitting off the DT bindings documentation.
Kind regards, Chris
>
> Does the above help?
^ permalink raw reply
* Re: [PATCH v9 3/4] dtc: Plugin and fixup support
From: Pantelis Antoniou @ 2016-11-24 14:23 UTC (permalink / raw)
To: Phil Elwell
Cc: David Gibson, Jon Loeliger, Grant Likely, Frank Rowand,
Rob Herring, Jan Luebbe, Sascha Hauer, Simon Glass, Maxime Ripard,
Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <2ad2929c-6e6a-4e31-0cca-cea2f11b14b1-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org>
Hi Phil,
> On Nov 24, 2016, at 16:14 , Phil Elwell <phil-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org> wrote:
>
> On 24/11/2016 13:58, Pantelis Antoniou wrote:
>> Hi Phil,
>>
>>> On Nov 24, 2016, at 15:49 , Phil Elwell <phil-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org> wrote:
>>>
>>> On 24/11/2016 12:31, Pantelis Antoniou wrote:
>>>> This patch enable the generation of symbols & local fixup information
>>>> for trees compiled with the -@ (--symbols) option.
>>>>
>>>> Using this patch labels in the tree and their users emit information
>>>> in __symbols__ and __local_fixups__ nodes.
>>>>
>>>> The __fixups__ node make possible the dynamic resolution of phandle
>>>> references which are present in the plugin tree but lie in the
>>>> tree that are applying the overlay against.
>>>>
>>>> While there is a new magic number for dynamic device tree/overlays blobs
>>>> it is by default disabled. This is in order to give time for DT blob
>>>> methods to be updated.
>>>>
>>>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
>>>> Signed-off-by: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>>>> Signed-off-by: Jan Luebbe <jlu-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>>> It's great to see this work about to come to fruition, but I have a
>>> reservation about this patch. Like previous versions, this
>>> implementation generates __fixups__, __local_fixups__ and __symbols__
>>> whenever the "-@" command line parameter is given; without it you get
>>> none. In my opinion, this logic causes the DTBs to be unnecessarily
>>> large with no obvious benefit.
>>>
>>> You can divide the compiled outputs from DTC into two categories - fully
>>> resolved DTBs (what I would call base DTBs), and overlays. Base DTBs
>>> should include __symbols__ to allow overlays to be applied, and it is
>>> right that this should be controlled by the "-@" flag, but since base
>>> DTBs are fully resolved I think there is no reason to include either
>>> __fixups__ or __local_fixups__. Therefore I think both kinds of fixups
>>> should only be omitted when the "/plugin/ " directive is used. This was
>>> the purpose of one of the patches I provided you with.
>>>
>> Yes, I’m quite aware of that. There is a reason for generating every
>> resolve node for the base tree. It contains information about the
>> dependencies of every hardware device due to phandle references.
>> We can utilize that to re-order the device probe order to eliminate
>> -EPROBE_DEFER cycles upon boot.
>>
>> It is important we get the core support in and then you can add extra
>> switches for every special platform case.
>>
>> What kind of problems do you have with larger device tree blobs?
>> I do carry a hashed phandle patch on my mainline tracking tree which
>> should help with larger DTBs.
> Early Raspberry Pi DT support used to load the DTBs to 0x100, Although
> the ARMv6/7 kernel starts at 0x8000, something (the decompressor?) would
> use the space between 0x4000 and the 0x8000, which gave us a practical
> limit of 16KB on the DTB size. This used to be sufficient for a base DTB
> and a few small overlays, but with the patch all Pi DTBs are over 16KB.
> The practical limit was overcome a long time ago, but it made me aware
> of the DTB contents, and there are may be some situations where it is
> desirable to reduce the footprint as much as possible.
>
I see now why you care about this. If you can regenerate your patch against
what I’ve posted I’d be happy to carry it along.
But DTBs are getting pretty large even without the extra fixup nodes. It is
not uncommon to see them at a few hundred KBs now.
> I would have thought that all DTBs already contain enough dependency
> information in the form of the phandles themselves. One of the first
> things the kernel does is to unflatten the DTB, and that is the obvious
> point to resolve the phandles and generate the necessary dependencies.
> Can you explain how both __fixups__ and __local_fixups__ aid this
> process? Ideally they wouldn't duplicate any information already in the
> tree, since then you have to cope with the possibility of malformed DTBs
> where the two don't actually match.
No, the DTBs by themselves do not contain enough information to build the
probe dependency graph, because phandles are simply converted to 32 bit
cell values on compile.
For instance take a case of one node using the other:
foo_label: foo { };
bar { use = <&foo_label>; };
A standard compile would generate a bar node as bar { use = <1>; };
While using the -@ switch you’d get a local ref that says that there is
a phandle cell value at offset 0 of bar/use property. Looking up the
phandle value you can see that this property references the foo node.
So when building the probe order graph the foo node should be probed
before bar.
>
> Phil
Regards
— Pantelis
--
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 V3 2/2] pinctrl: tegra: Add driver to configure voltage and power of io pads
From: Laxman Dewangan @ 2016-11-24 14:19 UTC (permalink / raw)
To: Linus Walleij, Mark Brown
Cc: Rob Herring, Stephen Warren, thierry.reding@gmail.com,
Mark Rutland, Alexandre Courbot, linux-gpio@vger.kernel.org,
devicetree@vger.kernel.org, linux-tegra@vger.kernel.org,
linux-kernel@vger.kernel.org, Joe Perches, Jon Hunter
In-Reply-To: <CACRpkdbp8_eR7PLoaX4AJbhcx_tYQjcS5U_hR3EU7b4dar3ckg@mail.gmail.com>
On Thursday 24 November 2016 07:41 PM, Linus Walleij wrote:
> On Wed, Nov 23, 2016 at 12:42 PM, Laxman Dewangan <ldewangan@nvidia.com> wrote:
>
>> Here, we need the regulator handle which can support the other regulator
>> APIs.
>>
>> In some of platforms, we do not use some of the io-pins and on this case, we
>> do not connect the IO rail supply for these pins. So this is like a hardware
>> optional.
>>
>> If the IO pins are used then it need to have the proper regulator handle.
>> Missing regulator handle on DT will be like that IO pads are not used.
> OK I buy that argument, unless Mark (Brown) has comments.
>
BTW, I resolved this in different way in V4 which I sent today, if
regulator_get() succeed for dummy then regulator_get_voltage() failed
and if it failed then assume that it is dummy and just ignore the
further handling based on regulator.
So when we really need the configuration based on voltage, we must
supply the regulator handle.
^ permalink raw reply
* Re: [PATCH RESEND 2/2] gpio: axp209: add pinctrl support
From: Linus Walleij @ 2016-11-24 14:17 UTC (permalink / raw)
To: Quentin Schulz
Cc: Alexandre Courbot, Rob Herring, Mark Rutland, Chen-Yu Tsai,
Maxime Ripard, linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <20161123141151.25315-3-quentin.schulz-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
On Wed, Nov 23, 2016 at 3:11 PM, Quentin Schulz
<quentin.schulz-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> The GPIOs present in the AXP209 PMIC have multiple functions. They
> typically allow a pin to be used as GPIO input or output and can also be
> used as ADC or regulator for example.[1]
>
> This adds the possibility to use all functions of the GPIOs present in
> the AXP209 PMIC thanks to pinctrl subsystem.
>
> [1] see registers 90H, 92H and 93H at
> http://dl.linux-sunxi.org/AXP/AXP209_Datasheet_v1.0en.pdf
>
> Signed-off-by: Quentin Schulz <quentin.schulz-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
I need Maxime's review on this patch.
> .../devicetree/bindings/gpio/gpio-axp209.txt | 28 +-
Also move the bindings to pinctrl/pinctrl-axp209.txt
> drivers/gpio/gpio-axp209.c | 551 ++++++++++++++++++---
Combined drivers should be in drivers/pinctrl/*.
Make a separate patch moving the driver to
drivers/pinctrl/pinctrl-axp209.c (remember -M to git format-patch)
augment Kconfig and Makefile in both subsystems and make
these patches on top of that.
I will deal with cross-merging the result between the GPIO
and pin control trees.
Yours,
Linus Walleij
--
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 v9 3/4] dtc: Plugin and fixup support
From: Phil Elwell @ 2016-11-24 14:14 UTC (permalink / raw)
To: Pantelis Antoniou
Cc: David Gibson, Jon Loeliger, Grant Likely, Frank Rowand,
Rob Herring, Jan Luebbe, Sascha Hauer, Simon Glass, Maxime Ripard,
Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <B341AF38-45C5-4954-B1E4-B89DED923929-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
On 24/11/2016 13:58, Pantelis Antoniou wrote:
> Hi Phil,
>
>> On Nov 24, 2016, at 15:49 , Phil Elwell <phil-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org> wrote:
>>
>> On 24/11/2016 12:31, Pantelis Antoniou wrote:
>>> This patch enable the generation of symbols & local fixup information
>>> for trees compiled with the -@ (--symbols) option.
>>>
>>> Using this patch labels in the tree and their users emit information
>>> in __symbols__ and __local_fixups__ nodes.
>>>
>>> The __fixups__ node make possible the dynamic resolution of phandle
>>> references which are present in the plugin tree but lie in the
>>> tree that are applying the overlay against.
>>>
>>> While there is a new magic number for dynamic device tree/overlays blobs
>>> it is by default disabled. This is in order to give time for DT blob
>>> methods to be updated.
>>>
>>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
>>> Signed-off-by: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>>> Signed-off-by: Jan Luebbe <jlu-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>> It's great to see this work about to come to fruition, but I have a
>> reservation about this patch. Like previous versions, this
>> implementation generates __fixups__, __local_fixups__ and __symbols__
>> whenever the "-@" command line parameter is given; without it you get
>> none. In my opinion, this logic causes the DTBs to be unnecessarily
>> large with no obvious benefit.
>>
>> You can divide the compiled outputs from DTC into two categories - fully
>> resolved DTBs (what I would call base DTBs), and overlays. Base DTBs
>> should include __symbols__ to allow overlays to be applied, and it is
>> right that this should be controlled by the "-@" flag, but since base
>> DTBs are fully resolved I think there is no reason to include either
>> __fixups__ or __local_fixups__. Therefore I think both kinds of fixups
>> should only be omitted when the "/plugin/ " directive is used. This was
>> the purpose of one of the patches I provided you with.
>>
> Yes, I’m quite aware of that. There is a reason for generating every
> resolve node for the base tree. It contains information about the
> dependencies of every hardware device due to phandle references.
> We can utilize that to re-order the device probe order to eliminate
> -EPROBE_DEFER cycles upon boot.
>
> It is important we get the core support in and then you can add extra
> switches for every special platform case.
>
> What kind of problems do you have with larger device tree blobs?
> I do carry a hashed phandle patch on my mainline tracking tree which
> should help with larger DTBs.
Early Raspberry Pi DT support used to load the DTBs to 0x100, Although
the ARMv6/7 kernel starts at 0x8000, something (the decompressor?) would
use the space between 0x4000 and the 0x8000, which gave us a practical
limit of 16KB on the DTB size. This used to be sufficient for a base DTB
and a few small overlays, but with the patch all Pi DTBs are over 16KB.
The practical limit was overcome a long time ago, but it made me aware
of the DTB contents, and there are may be some situations where it is
desirable to reduce the footprint as much as possible.
I would have thought that all DTBs already contain enough dependency
information in the form of the phandles themselves. One of the first
things the kernel does is to unflatten the DTB, and that is the obvious
point to resolve the phandles and generate the necessary dependencies.
Can you explain how both __fixups__ and __local_fixups__ aid this
process? Ideally they wouldn't duplicate any information already in the
tree, since then you have to cope with the possibility of malformed DTBs
where the two don't actually match.
Phil
^ permalink raw reply
* Re: [PATCH RESEND 1/2] gpio: axp209: use correct register for GPIO input status
From: Linus Walleij @ 2016-11-24 14:13 UTC (permalink / raw)
To: Quentin Schulz
Cc: Alexandre Courbot, Rob Herring, Mark Rutland, Chen-Yu Tsai,
Maxime Ripard, linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <20161123141151.25315-2-quentin.schulz-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
On Wed, Nov 23, 2016 at 3:11 PM, Quentin Schulz
<quentin.schulz-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> The GPIO input status was read from control register
> (AXP20X_GPIO[210]_CTRL) instead of status register (AXP20X_GPIO20_SS).
>
> Signed-off-by: Quentin Schulz <quentin.schulz-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Patch applied.
Yours,
Linus Walleij
--
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 V3 2/2] pinctrl: tegra: Add driver to configure voltage and power of io pads
From: Linus Walleij @ 2016-11-24 14:11 UTC (permalink / raw)
To: Laxman Dewangan, Mark Brown
Cc: Rob Herring, Stephen Warren, thierry.reding@gmail.com,
Mark Rutland, Alexandre Courbot, linux-gpio@vger.kernel.org,
devicetree@vger.kernel.org, linux-tegra@vger.kernel.org,
linux-kernel@vger.kernel.org, Joe Perches, Jon Hunter
In-Reply-To: <583580AD.60803@nvidia.com>
On Wed, Nov 23, 2016 at 12:42 PM, Laxman Dewangan <ldewangan@nvidia.com> wrote:
> Here, we need the regulator handle which can support the other regulator
> APIs.
>
> In some of platforms, we do not use some of the io-pins and on this case, we
> do not connect the IO rail supply for these pins. So this is like a hardware
> optional.
>
> If the IO pins are used then it need to have the proper regulator handle.
> Missing regulator handle on DT will be like that IO pads are not used.
OK I buy that argument, unless Mark (Brown) has comments.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 2/3] pinctrl: New driver for TI DA8XX/OMAP-L138/AM18XX pinconf
From: Linus Walleij @ 2016-11-24 14:05 UTC (permalink / raw)
To: David Lechner
Cc: Mark Rutland, devicetree@vger.kernel.org, Axel Haslam,
Kevin Hilman, Sekhar Nori, linux-kernel@vger.kernel.org,
linux-gpio@vger.kernel.org, Rob Herring, Alexandre Bailon,
Bartosz Gołaszewski, linux-arm-kernel@lists.infradead.org
In-Reply-To: <1479871767-20160-3-git-send-email-david@lechnology.com>
On Wed, Nov 23, 2016 at 4:29 AM, David Lechner <david@lechnology.com> wrote:
> This adds a new driver for pinconf on TI DA8XX/OMAP-L138/AM18XX. These
> SoCs have a separate controller for controlling pullup/pulldown groups.
>
> Signed-off-by: David Lechner <david@lechnology.com>
Nice and clean driver, resend with the minor fixes pointed out
by Sekhar and I'll merge it.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 1/3] devicetree: bindings: pinctrl: Add binding for ti,da850-pupd
From: Linus Walleij @ 2016-11-24 14:04 UTC (permalink / raw)
To: David Lechner
Cc: Rob Herring, Mark Rutland, Sekhar Nori, Kevin Hilman,
linux-gpio@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, Axel Haslam,
Alexandre Bailon, Bartosz Gołaszewski
In-Reply-To: <1479871767-20160-2-git-send-email-david@lechnology.com>
On Wed, Nov 23, 2016 at 4:29 AM, David Lechner <david@lechnology.com> wrote:
> Device-tree bindings for TI DA8XX/OMAP-L138/AM18XX pullup/pulldown
> pinconf controller.
>
> Signed-off-by: David Lechner <david@lechnology.com>
Looks good to me.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v9 3/4] dtc: Plugin and fixup support
From: Pantelis Antoniou @ 2016-11-24 13:58 UTC (permalink / raw)
To: Phil Elwell
Cc: David Gibson, Jon Loeliger, Grant Likely, Frank Rowand,
Rob Herring, Jan Luebbe, Sascha Hauer, Simon Glass, Maxime Ripard,
Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <dab7f38a-89c3-adbc-07a5-8ef8669ded42-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org>
Hi Phil,
> On Nov 24, 2016, at 15:49 , Phil Elwell <phil-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org> wrote:
>
> On 24/11/2016 12:31, Pantelis Antoniou wrote:
>> This patch enable the generation of symbols & local fixup information
>> for trees compiled with the -@ (--symbols) option.
>>
>> Using this patch labels in the tree and their users emit information
>> in __symbols__ and __local_fixups__ nodes.
>>
>> The __fixups__ node make possible the dynamic resolution of phandle
>> references which are present in the plugin tree but lie in the
>> tree that are applying the overlay against.
>>
>> While there is a new magic number for dynamic device tree/overlays blobs
>> it is by default disabled. This is in order to give time for DT blob
>> methods to be updated.
>>
>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
>> Signed-off-by: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>> Signed-off-by: Jan Luebbe <jlu-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> It's great to see this work about to come to fruition, but I have a
> reservation about this patch. Like previous versions, this
> implementation generates __fixups__, __local_fixups__ and __symbols__
> whenever the "-@" command line parameter is given; without it you get
> none. In my opinion, this logic causes the DTBs to be unnecessarily
> large with no obvious benefit.
>
> You can divide the compiled outputs from DTC into two categories - fully
> resolved DTBs (what I would call base DTBs), and overlays. Base DTBs
> should include __symbols__ to allow overlays to be applied, and it is
> right that this should be controlled by the "-@" flag, but since base
> DTBs are fully resolved I think there is no reason to include either
> __fixups__ or __local_fixups__. Therefore I think both kinds of fixups
> should only be omitted when the "/plugin/ " directive is used. This was
> the purpose of one of the patches I provided you with.
>
Yes, I’m quite aware of that. There is a reason for generating every
resolve node for the base tree. It contains information about the
dependencies of every hardware device due to phandle references.
We can utilize that to re-order the device probe order to eliminate
-EPROBE_DEFER cycles upon boot.
It is important we get the core support in and then you can add extra
switches for every special platform case.
What kind of problems do you have with larger device tree blobs?
I do carry a hashed phandle patch on my mainline tracking tree which
should help with larger DTBs.
> Phil Elwell, Raspberry Pi
Regards
— Pantelis
--
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 v9 3/4] dtc: Plugin and fixup support
From: Phil Elwell @ 2016-11-24 13:49 UTC (permalink / raw)
To: Pantelis Antoniou, David Gibson
Cc: Jon Loeliger, Grant Likely, Frank Rowand, Rob Herring, Jan Luebbe,
Sascha Hauer, Simon Glass, Maxime Ripard, Thomas Petazzoni,
Boris Brezillon, Antoine Tenart, Stephen Boyd,
Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1479990693-14260-4-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
On 24/11/2016 12:31, Pantelis Antoniou wrote:
> This patch enable the generation of symbols & local fixup information
> for trees compiled with the -@ (--symbols) option.
>
> Using this patch labels in the tree and their users emit information
> in __symbols__ and __local_fixups__ nodes.
>
> The __fixups__ node make possible the dynamic resolution of phandle
> references which are present in the plugin tree but lie in the
> tree that are applying the overlay against.
>
> While there is a new magic number for dynamic device tree/overlays blobs
> it is by default disabled. This is in order to give time for DT blob
> methods to be updated.
>
> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> Signed-off-by: Jan Luebbe <jlu-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
It's great to see this work about to come to fruition, but I have a
reservation about this patch. Like previous versions, this
implementation generates __fixups__, __local_fixups__ and __symbols__
whenever the "-@" command line parameter is given; without it you get
none. In my opinion, this logic causes the DTBs to be unnecessarily
large with no obvious benefit.
You can divide the compiled outputs from DTC into two categories - fully
resolved DTBs (what I would call base DTBs), and overlays. Base DTBs
should include __symbols__ to allow overlays to be applied, and it is
right that this should be controlled by the "-@" flag, but since base
DTBs are fully resolved I think there is no reason to include either
__fixups__ or __local_fixups__. Therefore I think both kinds of fixups
should only be omitted when the "/plugin/ " directive is used. This was
the purpose of one of the patches I provided you with.
Phil Elwell, Raspberry Pi
^ permalink raw reply
* Re: [PATCH 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell Xenon SDHC
From: Ziji Hu @ 2016-11-24 13:34 UTC (permalink / raw)
To: Ulf Hansson, Gregory CLEMENT
Cc: Adrian Hunter, linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Rob Herring,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Thomas Petazzoni,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Jimmy Xu, Jisheng Zhang, Nadav Haklai, Ryan Gao, Doug Jones,
Victor Gu, Wei(SOCP) Liu, Wilson Ding, Romain Perier
In-Reply-To: <CAPDyKFpkcoVMKbVOwjX1WDyNgc1vvUX60D6XRX6=YHGvkvHvnA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi Ulf,
Thanks a lot for the review.
On 2016/11/24 19:37, Ulf Hansson wrote:
> On 31 October 2016 at 12:09, Gregory CLEMENT
> <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
>> From: Ziji Hu <huziji-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
>>
>> Marvell Xenon eMMC/SD/SDIO Host Controller contains PHY.
>> Three types of PHYs are supported.
>>
>> Add support to multiple types of PHYs init and configuration.
>> Add register definitions of PHYs.
>>
>> Signed-off-by: Hu Ziji <huziji-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
>> Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>> ---
>> MAINTAINERS | 2 +-
>> drivers/mmc/host/Makefile | 2 +-
>> drivers/mmc/host/sdhci-xenon-phy.c | 1181 +++++++++++++++++++++++++++++-
>> drivers/mmc/host/sdhci-xenon-phy.h | 157 ++++-
>> drivers/mmc/host/sdhci-xenon.c | 4 +-
>> drivers/mmc/host/sdhci-xenon.h | 17 +-
>> 6 files changed, 1361 insertions(+), 2 deletions(-)
>> create mode 100644 drivers/mmc/host/sdhci-xenon-phy.c
>> create mode 100644 drivers/mmc/host/sdhci-xenon-phy.h
>
> Can you please consider to split this up somehow!? It would make it
> easier to review...
>
Sure. I will try to split them into smaller pieces.
> Anyway, allow me to provide some initial feedback, particularly around
> those things that Adrian and you requested for my input.
>
> [...]
>
>>
>> +
>> +static int __xenon_emmc_delay_adj_test(struct mmc_card *card)
>> +{
>> + int err;
>> + u8 *ext_csd = NULL;
>> +
>> + err = mmc_get_ext_csd(card, &ext_csd);
>> + kfree(ext_csd);
>
> Why do you read the ext csd here?
>
I would like to simply introduce the PHY setting of our SDHC.
The target of the PHY setting is to achieve a perfect sampling
point for transfers, during card initialization.
For HS200/HS400/SDR104 whose SDCLK is more than 50MHz, SDHC HW
will search for this sampling point with DLL's help.
For other speed mode whose SDLCK is less than or equals to 50MHz,
SW has to scan the PHY delay line to find out this perfect sampling
point. Our driver sends a command to verify a sampling point
in current environment.
As result, our SDHC driver has to implement the functionality to
send commands and check the results, in host layer.
If directly calling mmc_wait_for_cmd() is improper, could you please
give us some suggestions?
For eMMC, CMD8 is used to test current sampling point set in PHY.
>> +
>> + return err;
>> +}
>> +
>> +static int __xenon_sdio_delay_adj_test(struct mmc_card *card)
>> +{
>> + struct mmc_command cmd = {0};
>> + int err;
>> +
>> + cmd.opcode = SD_IO_RW_DIRECT;
>> + cmd.flags = MMC_RSP_R5 | MMC_CMD_AC;
>> +
>> + err = mmc_wait_for_cmd(card->host, &cmd, 0);
>> + if (err)
>> + return err;
>> +
>> + if (cmd.resp[0] & R5_ERROR)
>> + return -EIO;
>> + if (cmd.resp[0] & R5_FUNCTION_NUMBER)
>> + return -EINVAL;
>> + if (cmd.resp[0] & R5_OUT_OF_RANGE)
>> + return -ERANGE;
>> + return 0;
>
> No thanks! MMC/SD/SDIO protocol code belongs in the core.
>
For SDIO, SD_IO_RW_DIRECT command is sent to test current sampling point
in PHY.
Please help provide some suggestion to implement the command transfer.
>> +}
>> +
>> +static int __xenon_sd_delay_adj_test(struct mmc_card *card)
>> +{
>> + struct mmc_command cmd = {0};
>> + int err;
>> +
>> + cmd.opcode = MMC_SEND_STATUS;
>> + cmd.arg = card->rca << 16;
>> + cmd.flags = MMC_RSP_R1 | MMC_CMD_AC;
>> +
>> + err = mmc_wait_for_cmd(card->host, &cmd, 0);
>> + return err;
>
> No thanks! MMC/SD/SDIO protocol code belongs in the core.
>
>> +}
>> +
>
> [...]
>
>> +int xenon_phy_adj(struct sdhci_host *host, struct mmc_ios *ios)
>> +{
>> + struct mmc_host *mmc = host->mmc;
>> + struct mmc_card *card;
>> + int ret = 0;
>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
>> +
>> + if (!host->clock) {
>> + priv->clock = 0;
>> + return 0;
>> + }
>> +
>> + /*
>> + * The timing, frequency or bus width is changed,
>> + * better to set eMMC PHY based on current setting
>> + * and adjust Xenon SDHC delay.
>> + */
>> + if ((host->clock == priv->clock) &&
>> + (ios->bus_width == priv->bus_width) &&
>> + (ios->timing == priv->timing))
>> + return 0;
>> +
>> + xenon_phy_set(host, ios->timing);
>> +
>> + /* Update the record */
>> + priv->bus_width = ios->bus_width;
>> + /* Temp stage from HS200 to HS400 */
>> + if (((priv->timing == MMC_TIMING_MMC_HS200) &&
>> + (ios->timing == MMC_TIMING_MMC_HS)) ||
>> + ((ios->timing == MMC_TIMING_MMC_HS) &&
>> + (priv->clock > host->clock))) {
>> + priv->timing = ios->timing;
>> + priv->clock = host->clock;
>> + return 0;
>> + }
>> + /*
>> + * Skip temp stages from HS400 t0 HS200:
>> + * from 200MHz to 52MHz in HS400
>> + * from HS400 to HS DDR in 52MHz
>> + * from HS DDR to HS in 52MHz
>> + * from HS to HS200 in 52MHz
>> + */
>> + if (((priv->timing == MMC_TIMING_MMC_HS400) &&
>> + ((host->clock == MMC_HIGH_52_MAX_DTR) ||
>> + (ios->timing == MMC_TIMING_MMC_DDR52))) ||
>> + ((priv->timing == MMC_TIMING_MMC_DDR52) &&
>> + (ios->timing == MMC_TIMING_MMC_HS)) ||
>> + ((ios->timing == MMC_TIMING_MMC_HS200) &&
>> + (ios->clock == MMC_HIGH_52_MAX_DTR))) {
>> + priv->timing = ios->timing;
>> + priv->clock = host->clock;
>> + return 0;
>> + }
>> + priv->timing = ios->timing;
>> + priv->clock = host->clock;
>> +
>> + /* Legacy mode is a special case */
>> + if (ios->timing == MMC_TIMING_LEGACY)
>> + return 0;
>> +
>> + if (mmc->card)
>> + card = mmc->card;
>> + else
>> + /*
>> + * Only valid during initialization
>> + * before mmc->card is set
>> + */
>> + card = priv->card_candidate;
>> + if (unlikely(!card)) {
>> + dev_warn(mmc_dev(mmc), "card is not present\n");
>> + return -EINVAL;
>> + }
>
> That your host need to hold a copy of the card pointer, tells me that
> something is not really correct.
>
> I might be wrong, if this turns out to be a special case, but I doubt
> it. Although, if it *is* a special such case, we shall most likely try
> to extend the the mmc core layer instead of adding all these hacks in
> your host driver.
>
This card pointer copies the temporary structure mmc_card
used in mmc_init_card(), mmc_sd_init_card() and mmc_sdio_init_card().
Since we call mmc_wait_for_cmd() to send test commands, we need a copy
of that temporary mmc_card here in our host driver.
During PHY setting in card initialization, mmc_host->card is not updated
yet with that temporary mmc_card. Thus we are not able to directly use
mmc_host->card. Instead, this card pointer is introduced to enable
mmc_wait_for_cmd().
If we can improve our host driver to send test commands without mmc_card,
this card pointer can be removed.
Could you please share your opinion please?
> [...]
>
> Another suggestion of a general improvement; could you perhaps try to
> add some brief information about what goes on in function headers.
> Perhaps that could help to more easily understand things.
>
Sorry about any inconvenience. Most of the functions here are our host specific.
It is really difficult to understand them without proper comment.
I will add more information.
Thank you.
Best regards,
Hu Ziji
> Kind regards
> Uffe
>
--
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 6/10] mmc: sdhci-xenon: Add Marvell Xenon SDHC core functionality
From: Ulf Hansson @ 2016-11-24 13:34 UTC (permalink / raw)
To: Ziji Hu, Adrian Hunter
Cc: Gregory CLEMENT, linux-mmc@vger.kernel.org, Jason Cooper,
Andrew Lunn, Sebastian Hesselbarth, Rob Herring,
devicetree@vger.kernel.org, Thomas Petazzoni,
linux-arm-kernel@lists.infradead.org, Jimmy Xu, Jisheng Zhang,
Nadav Haklai, Ryan Gao, Doug Jones, Victor Gu, Wei(SOCP) Liu,
Wilson Ding, Romain Perier
In-Reply-To: <dd230463-04f6-df31-7056-1a185eb6cfc7@marvell.com>
On 24 November 2016 at 13:41, Ziji Hu <huziji@marvell.com> wrote:
> Hi Ulf,
>
> On 2016/11/24 18:43, Ulf Hansson wrote:
>> On 31 October 2016 at 12:09, Gregory CLEMENT
>> <gregory.clement@free-electrons.com> wrote:
>>> From: Ziji Hu <huziji@marvell.com>
>>>
> <snip>
>>> +static int xenon_emmc_signal_voltage_switch(struct mmc_host *mmc,
>>> + struct mmc_ios *ios)
>>> +{
>>> + unsigned char voltage = ios->signal_voltage;
>>> +
>>> + if ((voltage == MMC_SIGNAL_VOLTAGE_330) ||
>>> + (voltage == MMC_SIGNAL_VOLTAGE_180))
>>> + return __emmc_signal_voltage_switch(mmc, voltage);
>>> +
>>> + dev_err(mmc_dev(mmc), "Unsupported signal voltage: %d\n",
>>> + voltage);
>>> + return -EINVAL;
>>
>> This wrapper function seems unnessarry. It only adds a dev_err(), so
>> then might as well do that in __emmc_signal_voltage_switch().
>>
> Sure. Will merge it back to __emmc_signal_voltage_switch().
>
>>> +}
>>> +
>>> +static int xenon_start_signal_voltage_switch(struct mmc_host *mmc,
>>> + struct mmc_ios *ios)
>>> +{
>>> + struct sdhci_host *host = mmc_priv(mmc);
>>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
>>> +
>>> + /*
>>> + * Before SD/SDIO set signal voltage, SD bus clock should be
>>> + * disabled. However, sdhci_set_clock will also disable the Internal
>>> + * clock in mmc_set_signal_voltage().
>>
>> If that's the case then that is wrong in the generic sdhci code.
>> What's the reason why it can't be fixed there instead of having this
>> workaround?
>>
> In my very own opinion, SD Spec doesn't specify whether SDCLK should be
> enabled or not during power setting.
> Enabling SDCLK might be a special condition only required by our SDHC.
> I try to avoid breaking other vendors' SDHC functionality
> if their SDHCs require SDCLK disabled.
> Thus I prefer to keep it inside our SDHC driver.
I let Adrian comment on this.
For sure we should avoid breaking other sdhci variant, but on the
other hand *if* the generic code is wrong we should fix it!
>
>>> + * If Internal clock is disabled, the 3.3V/1.8V bit can not be updated.
>>> + * Thus here manually enable internal clock.
>>> + *
>>> + * After switch completes, it is unnecessary to disable internal clock,
>>> + * since keeping internal clock active obeys SD spec.
>>> + */
>>> + enable_xenon_internal_clk(host);
>>> +
>>> + if (priv->emmc_slot)
>>> + return xenon_emmc_signal_voltage_switch(mmc, ios);
>>> +
>>> + return sdhci_start_signal_voltage_switch(mmc, ios);
>>> +}
>>> +
>>> +/*
>>> + * After determining which slot is used for SDIO,
>>> + * some additional task is required.
>>> + */
>>> +static void xenon_init_card(struct mmc_host *mmc, struct mmc_card *card)
>>> +{
>>> + struct sdhci_host *host = mmc_priv(mmc);
>>> + u32 reg;
>>> + u8 slot_idx;
>>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
>>> +
>>> + /* Link the card for delay adjustment */
>>> + priv->card_candidate = card;
>>> + /* Set tuning functionality of this slot */
>>> + xenon_slot_tuning_setup(host);
>>
>> This looks weird. I assume this can be done as a part of the regular
>> tuning seqeunce!?
>>
> It is our SDHC specific preparation prior to tuning, rather than a
> standard step in spec.
> Thus I leave it inside our driver.
My point is that this isn't the purpose of ->init_card(). thus you are
abusing it.
Try to make it work in another way, please. I think you can.
>
>>> +
>>> + slot_idx = priv->slot_idx;
>>> + if (!mmc_card_sdio(card)) {
>>> + /* Clear SDIO Card Inserted indication */
>>
>> Why do you need this?
>>
>> If you need to reset this, I think it's better to do it from
>> ->set_ios() at MMC_POWER_OFF.
>>
> This field indicates SDIO card and controls async interrupt feature
> of SDIO in our SDHC.
> This async interrupt feature is enabled when SDIO card is inserted.
> It should be disabled if SD card is inserted instead.
What do you mean by SDIO async interupts? Are you talking about SDIO
irqs on DAT1 line?
Those is supposed to be enabled when someone explicitly requests them,
not when the card is inserted.
In other words when an SDIO func driver have called sdio_claim_irq().
Moreover, we have ->enable_sdio_irq() ops that deals with this.
[...]
>>> +
>>> + /*
>>> + * Xenon Specific property:
>>> + * emmc: explicitly indicate whether this slot is for eMMC
>>> + * slotno: the index of slot. Refer to SDHC_SYS_CFG_INFO register
>>> + * tun-count: the interval between re-tuning
>>> + * PHY type: "sdhc phy", "emmc phy 5.0" or "emmc phy 5.1"
>>> + */
>>> + if (of_property_read_bool(np, "marvell,xenon-emmc"))
>>> + priv->emmc_slot = true;
>>
>> So, you need this because of the eMMC voltage switch behaviour, right?
>>
>> Then I would rather like to describe this a generic DT bindings for
>> the eMMC voltage level support. There have acutally been some earlier
>> discussions for this, but we haven't yet made some changes.
>>
>> I think what is missing is a mmc-ddr-3_3v DT binding, which when set,
>> allows the host driver to accept I/O voltage switches to 3.3V. If not
>> supported the ->start_signal_voltage_switch() ops may return -EINVAL.
>> This would inform the mmc core to move on to the next supported
>> voltage level. There might be some minor additional changes to the mmc
>> card initialization sequence, but those should be simple.
>>
>> I can help out to look into this, unless you want to do it yourself of course!?
>>
> Yes. One of the reasons is to provide eMMC specific voltage setting.
> But in my very own opinion, it should be irrelevant to voltage level.
> The eMMC voltage setting on our SDHC is different from SD/SDIO voltage switch.
> It will become more complex with different SOC implementation details.
Got it. Although I think we can cope with that fine just by using the
different SD/eMMC speed modes settings defined in DT (or from the
SDHCI caps register)
> Unfortunately, MMC driver cannot determine the card type yet when eMMC voltage
> setting should be executed.
> Thus an flag is required here to tell driver to execute eMMC voltage setting.
>
> Besides, additional eMMC specific settings might be implemented in future, besides
> voltage setting. Most of them should be completed before MMC driver recognizes the
> card type. Thus I have to keep this flag to indicate current SDHC is for eMMC.
I doubt you will need a generic "eMMC" flag, but let's see when we go forward.
Currently it's clear you don't need such a flag, so I will submit a
change adding a DT binding for "mmc-ddr-3_3v" then we can take it from
there, to see if it suits your needs.
[...]
Kind regards
Uffe
^ 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