* [PATCH v8 10/21] clk: tegra: clk-super: Add restore-context support
From: Sowjanya Komatineni @ 2019-08-08 23:46 UTC (permalink / raw)
To: thierry.reding, jonathanh, tglx, jason, marc.zyngier,
linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, skomatineni, linux-tegra, linux-kernel,
mperttunen, spatra, robh+dt, digetx, devicetree, rjw,
viresh.kumar, linux-pm
In-Reply-To: <1565308020-31952-1-git-send-email-skomatineni@nvidia.com>
This patch implements restore_context for clk_super_mux and clk_super.
During system supend, core power goes off the and context of Tegra
CAR registers is lost.
So on system resume, context of super clock registers are restored
to have them in same state as before suspend.
Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
drivers/clk/tegra/clk-super.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/drivers/clk/tegra/clk-super.c b/drivers/clk/tegra/clk-super.c
index e2a1e95a8db7..74c9e913e41c 100644
--- a/drivers/clk/tegra/clk-super.c
+++ b/drivers/clk/tegra/clk-super.c
@@ -124,9 +124,18 @@ static int clk_super_set_parent(struct clk_hw *hw, u8 index)
return err;
}
+static void clk_super_mux_restore_context(struct clk_hw *hw)
+{
+ struct clk_hw *parent = clk_hw_get_parent(hw);
+ int parent_id = clk_hw_get_parent_index(hw, parent);
+
+ clk_super_set_parent(hw, parent_id);
+}
+
static const struct clk_ops tegra_clk_super_mux_ops = {
.get_parent = clk_super_get_parent,
.set_parent = clk_super_set_parent,
+ .restore_context = clk_super_mux_restore_context,
};
static long clk_super_round_rate(struct clk_hw *hw, unsigned long rate,
@@ -162,12 +171,24 @@ static int clk_super_set_rate(struct clk_hw *hw, unsigned long rate,
return super->div_ops->set_rate(div_hw, rate, parent_rate);
}
+static void clk_super_restore_context(struct clk_hw *hw)
+{
+ struct tegra_clk_super_mux *super = to_clk_super_mux(hw);
+ struct clk_hw *div_hw = &super->frac_div.hw;
+ struct clk_hw *parent = clk_hw_get_parent(hw);
+ int parent_id = clk_hw_get_parent_index(hw, parent);
+
+ super->div_ops->restore_context(div_hw);
+ clk_super_set_parent(hw, parent_id);
+}
+
const struct clk_ops tegra_clk_super_ops = {
.get_parent = clk_super_get_parent,
.set_parent = clk_super_set_parent,
.set_rate = clk_super_set_rate,
.round_rate = clk_super_round_rate,
.recalc_rate = clk_super_recalc_rate,
+ .restore_context = clk_super_restore_context,
};
struct clk *tegra_clk_register_super_mux(const char *name,
--
2.7.4
^ permalink raw reply related
* [PATCH v8 06/21] clk: tegra: Support for OSC context save and restore
From: Sowjanya Komatineni @ 2019-08-08 23:46 UTC (permalink / raw)
To: thierry.reding, jonathanh, tglx, jason, marc.zyngier,
linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, skomatineni, linux-tegra, linux-kernel,
mperttunen, spatra, robh+dt, digetx, devicetree, rjw,
viresh.kumar, linux-pm
In-Reply-To: <1565308020-31952-1-git-send-email-skomatineni@nvidia.com>
This patch adds support for saving OSC clock frequency and the
drive-strength during OSC clock init and creates an API to restore
OSC control register value from the saved context.
This API is invoked by Tegra210 clock driver during system resume
to restore the OSC clock settings.
Acked-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
drivers/clk/tegra/clk-tegra-fixed.c | 15 +++++++++++++++
drivers/clk/tegra/clk.h | 1 +
2 files changed, 16 insertions(+)
diff --git a/drivers/clk/tegra/clk-tegra-fixed.c b/drivers/clk/tegra/clk-tegra-fixed.c
index 8d91b2b191cf..7c6c8abfcde6 100644
--- a/drivers/clk/tegra/clk-tegra-fixed.c
+++ b/drivers/clk/tegra/clk-tegra-fixed.c
@@ -17,6 +17,10 @@
#define OSC_CTRL 0x50
#define OSC_CTRL_OSC_FREQ_SHIFT 28
#define OSC_CTRL_PLL_REF_DIV_SHIFT 26
+#define OSC_CTRL_MASK (0x3f2 | \
+ (0xf << OSC_CTRL_OSC_FREQ_SHIFT))
+
+static u32 osc_ctrl_ctx;
int __init tegra_osc_clk_init(void __iomem *clk_base, struct tegra_clk *clks,
unsigned long *input_freqs, unsigned int num,
@@ -29,6 +33,7 @@ int __init tegra_osc_clk_init(void __iomem *clk_base, struct tegra_clk *clks,
unsigned osc_idx;
val = readl_relaxed(clk_base + OSC_CTRL);
+ osc_ctrl_ctx = val & OSC_CTRL_MASK;
osc_idx = val >> OSC_CTRL_OSC_FREQ_SHIFT;
if (osc_idx < num)
@@ -96,3 +101,13 @@ void __init tegra_fixed_clk_init(struct tegra_clk *tegra_clks)
*dt_clk = clk;
}
}
+
+void tegra_clk_osc_resume(void __iomem *clk_base)
+{
+ u32 val;
+
+ val = readl_relaxed(clk_base + OSC_CTRL) & ~OSC_CTRL_MASK;
+ val |= osc_ctrl_ctx;
+ writel_relaxed(val, clk_base + OSC_CTRL);
+ fence_udelay(2, clk_base);
+}
diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
index dc546292e030..8a9af45b6084 100644
--- a/drivers/clk/tegra/clk.h
+++ b/drivers/clk/tegra/clk.h
@@ -837,6 +837,7 @@ u16 tegra_pll_get_fixed_mdiv(struct clk_hw *hw, unsigned long input_rate);
int tegra_pll_p_div_to_hw(struct tegra_clk_pll *pll, u8 p_div);
int div_frac_get(unsigned long rate, unsigned parent_rate, u8 width,
u8 frac_width, u8 flags);
+void tegra_clk_osc_resume(void __iomem *clk_base);
/* Combined read fence with delay */
--
2.7.4
^ permalink raw reply related
* [PATCH v8 04/21] clk: tegra: pllout: Save and restore pllout context
From: Sowjanya Komatineni @ 2019-08-08 23:46 UTC (permalink / raw)
To: thierry.reding, jonathanh, tglx, jason, marc.zyngier,
linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, skomatineni, linux-tegra, linux-kernel,
mperttunen, spatra, robh+dt, digetx, devicetree, rjw,
viresh.kumar, linux-pm
In-Reply-To: <1565308020-31952-1-git-send-email-skomatineni@nvidia.com>
This patch implements save and restore of pllout context.
During system suspend, core power goes off and looses the settings
of the Tegra CAR controller registers.
So during suspend entry the state of pllout is saved and on resume
it is restored back to have pllout in same state as before suspend.
pllout rate is saved and restore in clock divider so it will be at
same rate as before suspend when pllout state is restored.
Acked-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
drivers/clk/tegra/clk-pll-out.c | 9 +++++++++
drivers/clk/tegra/clk-tegra210.c | 3 ++-
drivers/clk/tegra/clk.h | 6 ++++++
3 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/tegra/clk-pll-out.c b/drivers/clk/tegra/clk-pll-out.c
index 35f2bf00e1e6..d8bf89a81e6d 100644
--- a/drivers/clk/tegra/clk-pll-out.c
+++ b/drivers/clk/tegra/clk-pll-out.c
@@ -69,10 +69,19 @@ static void clk_pll_out_disable(struct clk_hw *hw)
spin_unlock_irqrestore(pll_out->lock, flags);
}
+static void tegra_clk_pll_out_restore_context(struct clk_hw *hw)
+{
+ if (!__clk_get_enable_count(hw->clk))
+ clk_pll_out_disable(hw);
+ else
+ clk_pll_out_enable(hw);
+}
+
const struct clk_ops tegra_clk_pll_out_ops = {
.is_enabled = clk_pll_out_is_enabled,
.enable = clk_pll_out_enable,
.disable = clk_pll_out_disable,
+ .restore_context = tegra_clk_pll_out_restore_context,
};
struct clk *tegra_clk_register_pll_out(const char *name,
diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c
index df172d5772d7..4721ee030d1c 100644
--- a/drivers/clk/tegra/clk-tegra210.c
+++ b/drivers/clk/tegra/clk-tegra210.c
@@ -3200,7 +3200,8 @@ static void __init tegra210_pll_init(void __iomem *clk_base,
8, 8, 1, NULL);
clk = tegra_clk_register_pll_out("pll_re_out1", "pll_re_out1_div",
clk_base + PLLRE_OUT1, 1, 0,
- CLK_SET_RATE_PARENT, 0, NULL);
+ CLK_SET_RATE_PARENT, TEGRA_PLLRE_OUT,
+ NULL);
clks[TEGRA210_CLK_PLL_RE_OUT1] = clk;
/* PLLE */
diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
index 905bf1096558..a464524fbc90 100644
--- a/drivers/clk/tegra/clk.h
+++ b/drivers/clk/tegra/clk.h
@@ -437,6 +437,10 @@ struct clk *tegra_clk_register_pllu_tegra210(const char *name,
* @rst_bit_idx: bit to reset PLL divider
* @lock: register lock
* @flags: hardware-specific flags
+ *
+ * Flags:
+ * TEGRA_PLLRE_OUT - This flag indicates that it is PLLRE_OUT and is used to
+ * identify PLLRE_OUT during clk_pll_out save and restore.
*/
struct tegra_clk_pll_out {
struct clk_hw hw;
@@ -447,6 +451,8 @@ struct tegra_clk_pll_out {
u8 flags;
};
+#define TEGRA_PLLRE_OUT BIT(0)
+
#define to_clk_pll_out(_hw) container_of(_hw, struct tegra_clk_pll_out, hw)
extern const struct clk_ops tegra_clk_pll_out_ops;
--
2.7.4
^ permalink raw reply related
* [PATCH v8 11/21] clk: tegra: clk-dfll: Add suspend and resume support
From: Sowjanya Komatineni @ 2019-08-08 23:46 UTC (permalink / raw)
To: thierry.reding, jonathanh, tglx, jason, marc.zyngier,
linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, skomatineni, linux-tegra, linux-kernel,
mperttunen, spatra, robh+dt, digetx, devicetree, rjw,
viresh.kumar, linux-pm
In-Reply-To: <1565308020-31952-1-git-send-email-skomatineni@nvidia.com>
This patch implements DFLL suspend and resume operation.
During system suspend entry, CPU clock will switch CPU to safe
clock source of PLLP and disables DFLL clock output.
DFLL driver suspend confirms DFLL disable state and errors out on
being active.
DFLL is re-initialized during the DFLL driver resume as it goes
through complete reset during suspend entry.
Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
drivers/clk/tegra/clk-dfll.c | 56 ++++++++++++++++++++++++++++++
drivers/clk/tegra/clk-dfll.h | 2 ++
drivers/clk/tegra/clk-tegra124-dfll-fcpu.c | 1 +
3 files changed, 59 insertions(+)
diff --git a/drivers/clk/tegra/clk-dfll.c b/drivers/clk/tegra/clk-dfll.c
index f8688c2ddf1a..eb298a5d7be9 100644
--- a/drivers/clk/tegra/clk-dfll.c
+++ b/drivers/clk/tegra/clk-dfll.c
@@ -1487,6 +1487,7 @@ static int dfll_init(struct tegra_dfll *td)
td->last_unrounded_rate = 0;
pm_runtime_enable(td->dev);
+ pm_runtime_irq_safe(td->dev);
pm_runtime_get_sync(td->dev);
dfll_set_mode(td, DFLL_DISABLED);
@@ -1513,6 +1514,61 @@ static int dfll_init(struct tegra_dfll *td)
return ret;
}
+/**
+ * tegra_dfll_suspend - check DFLL is disabled
+ * @dev: DFLL device *
+ *
+ * DFLL clock should be disabled by the CPUFreq driver. So, make
+ * sure it is disabled and disable all clocks needed by the DFLL.
+ */
+int tegra_dfll_suspend(struct device *dev)
+{
+ struct tegra_dfll *td = dev_get_drvdata(dev);
+
+ if (dfll_is_running(td)) {
+ dev_err(td->dev, "dfll is enabled while shouldn't be\n");
+ return -EBUSY;
+ }
+
+ reset_control_assert(td->dvco_rst);
+
+ return 0;
+}
+EXPORT_SYMBOL(tegra_dfll_suspend);
+
+/**
+ * tegra_dfll_resume - reinitialize DFLL on resume
+ * @dev: DFLL instance
+ *
+ * DFLL is disabled and reset during suspend and resume.
+ * So, reinitialize the DFLL IP block back for use.
+ * DFLL clock is enabled later in closed loop mode by CPUFreq
+ * driver before switching its clock source to DFLL output.
+ */
+int tegra_dfll_resume(struct device *dev)
+{
+ struct tegra_dfll *td = dev_get_drvdata(dev);
+
+ reset_control_deassert(td->dvco_rst);
+
+ pm_runtime_get_sync(td->dev);
+
+ dfll_set_mode(td, DFLL_DISABLED);
+ dfll_set_default_params(td);
+
+ if (td->soc->init_clock_trimmers)
+ td->soc->init_clock_trimmers();
+
+ dfll_set_open_loop_config(td);
+
+ dfll_init_out_if(td);
+
+ pm_runtime_put_sync(td->dev);
+
+ return 0;
+}
+EXPORT_SYMBOL(tegra_dfll_resume);
+
/*
* DT data fetch
*/
diff --git a/drivers/clk/tegra/clk-dfll.h b/drivers/clk/tegra/clk-dfll.h
index 1b14ebe7268b..fb209eb5f365 100644
--- a/drivers/clk/tegra/clk-dfll.h
+++ b/drivers/clk/tegra/clk-dfll.h
@@ -42,5 +42,7 @@ int tegra_dfll_register(struct platform_device *pdev,
struct tegra_dfll_soc_data *tegra_dfll_unregister(struct platform_device *pdev);
int tegra_dfll_runtime_suspend(struct device *dev);
int tegra_dfll_runtime_resume(struct device *dev);
+int tegra_dfll_suspend(struct device *dev);
+int tegra_dfll_resume(struct device *dev);
#endif /* __DRIVERS_CLK_TEGRA_CLK_DFLL_H */
diff --git a/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c b/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
index e84b6d52cbbd..2ac2679d696d 100644
--- a/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
+++ b/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
@@ -631,6 +631,7 @@ static int tegra124_dfll_fcpu_remove(struct platform_device *pdev)
static const struct dev_pm_ops tegra124_dfll_pm_ops = {
SET_RUNTIME_PM_OPS(tegra_dfll_runtime_suspend,
tegra_dfll_runtime_resume, NULL)
+ SET_SYSTEM_SLEEP_PM_OPS(tegra_dfll_suspend, tegra_dfll_resume)
};
static struct platform_driver tegra124_dfll_fcpu_driver = {
--
2.7.4
^ permalink raw reply related
* [PATCH v8 13/21] clk: tegra210: Use fence_udelay during PLLU init
From: Sowjanya Komatineni @ 2019-08-08 23:46 UTC (permalink / raw)
To: thierry.reding, jonathanh, tglx, jason, marc.zyngier,
linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, skomatineni, linux-tegra, linux-kernel,
mperttunen, spatra, robh+dt, digetx, devicetree, rjw,
viresh.kumar, linux-pm
In-Reply-To: <1565308020-31952-1-git-send-email-skomatineni@nvidia.com>
This patch uses fence_udelay rather than udelay during PLLU
initialization to ensure writes to clock registers happens before
waiting for specified delay.
Acked-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
drivers/clk/tegra/clk-tegra210.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c
index 4721ee030d1c..998bf60b219a 100644
--- a/drivers/clk/tegra/clk-tegra210.c
+++ b/drivers/clk/tegra/clk-tegra210.c
@@ -2841,7 +2841,7 @@ static int tegra210_enable_pllu(void)
reg = readl_relaxed(clk_base + pllu.params->ext_misc_reg[0]);
reg &= ~BIT(pllu.params->iddq_bit_idx);
writel_relaxed(reg, clk_base + pllu.params->ext_misc_reg[0]);
- udelay(5);
+ fence_udelay(5, clk_base);
reg = readl_relaxed(clk_base + PLLU_BASE);
reg &= ~GENMASK(20, 0);
@@ -2849,7 +2849,7 @@ static int tegra210_enable_pllu(void)
reg |= fentry->n << 8;
reg |= fentry->p << 16;
writel(reg, clk_base + PLLU_BASE);
- udelay(1);
+ fence_udelay(1, clk_base);
reg |= PLL_ENABLE;
writel(reg, clk_base + PLLU_BASE);
@@ -2895,12 +2895,12 @@ static int tegra210_init_pllu(void)
reg = readl_relaxed(clk_base + XUSB_PLL_CFG0);
reg &= ~XUSB_PLL_CFG0_PLLU_LOCK_DLY_MASK;
writel_relaxed(reg, clk_base + XUSB_PLL_CFG0);
- udelay(1);
+ fence_udelay(1, clk_base);
reg = readl_relaxed(clk_base + PLLU_HW_PWRDN_CFG0);
reg |= PLLU_HW_PWRDN_CFG0_SEQ_ENABLE;
writel_relaxed(reg, clk_base + PLLU_HW_PWRDN_CFG0);
- udelay(1);
+ fence_udelay(1, clk_base);
reg = readl_relaxed(clk_base + PLLU_BASE);
reg &= ~PLLU_BASE_CLKENABLE_USB;
--
2.7.4
^ permalink raw reply related
* [PATCH v8 05/21] clk: tegra: pll: Save and restore pll context
From: Sowjanya Komatineni @ 2019-08-08 23:46 UTC (permalink / raw)
To: thierry.reding, jonathanh, tglx, jason, marc.zyngier,
linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, skomatineni, linux-tegra, linux-kernel,
mperttunen, spatra, robh+dt, digetx, devicetree, rjw,
viresh.kumar, linux-pm
In-Reply-To: <1565308020-31952-1-git-send-email-skomatineni@nvidia.com>
This patch implements save and restore of PLL context.
During system suspend, core power goes off and looses the settings
of the Tegra CAR controller registers.
So during suspend entry pll context is stored and on resume it is
restored back along with its state.
Acked-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
drivers/clk/tegra/clk-pll.c | 88 ++++++++++++++++++++++++++++-----------------
drivers/clk/tegra/clk.h | 2 ++
2 files changed, 58 insertions(+), 32 deletions(-)
diff --git a/drivers/clk/tegra/clk-pll.c b/drivers/clk/tegra/clk-pll.c
index 1583f5fc992f..e52add2bbdbb 100644
--- a/drivers/clk/tegra/clk-pll.c
+++ b/drivers/clk/tegra/clk-pll.c
@@ -1008,6 +1008,28 @@ static unsigned long clk_plle_recalc_rate(struct clk_hw *hw,
return rate;
}
+static void tegra_clk_pll_restore_context(struct clk_hw *hw)
+{
+ struct tegra_clk_pll *pll = to_clk_pll(hw);
+ struct clk_hw *parent = clk_hw_get_parent(hw);
+ unsigned long parent_rate = clk_hw_get_rate(parent);
+ unsigned long rate = clk_hw_get_rate(hw);
+ u32 val;
+
+ if (clk_pll_is_enabled(hw))
+ return;
+
+ if (pll->params->set_defaults)
+ pll->params->set_defaults(pll);
+
+ clk_pll_set_rate(hw, rate, parent_rate);
+
+ if (!__clk_get_enable_count(hw->clk))
+ clk_pll_disable(hw);
+ else
+ clk_pll_enable(hw);
+}
+
const struct clk_ops tegra_clk_pll_ops = {
.is_enabled = clk_pll_is_enabled,
.enable = clk_pll_enable,
@@ -1015,6 +1037,7 @@ const struct clk_ops tegra_clk_pll_ops = {
.recalc_rate = clk_pll_recalc_rate,
.round_rate = clk_pll_round_rate,
.set_rate = clk_pll_set_rate,
+ .restore_context = tegra_clk_pll_restore_context,
};
const struct clk_ops tegra_clk_plle_ops = {
@@ -1802,6 +1825,27 @@ static int clk_pllu_tegra114_enable(struct clk_hw *hw)
return ret;
}
+
+static void _clk_plle_tegra_init_parent(struct tegra_clk_pll *pll)
+{
+ u32 val, val_aux;
+
+ /* ensure parent is set to pll_ref */
+ val = pll_readl_base(pll);
+ val_aux = pll_readl(pll->params->aux_reg, pll);
+
+ if (val & PLL_BASE_ENABLE) {
+ if ((val_aux & PLLE_AUX_PLLRE_SEL) ||
+ (val_aux & PLLE_AUX_PLLP_SEL))
+ WARN(1, "pll_e enabled with unsupported parent %s\n",
+ (val_aux & PLLE_AUX_PLLP_SEL) ? "pllp_out0" :
+ "pll_re_vco");
+ } else {
+ val_aux &= ~(PLLE_AUX_PLLRE_SEL | PLLE_AUX_PLLP_SEL);
+ pll_writel(val_aux, pll->params->aux_reg, pll);
+ fence_udelay(1, pll->clk_base);
+ }
+}
#endif
static struct tegra_clk_pll *_tegra_init_pll(void __iomem *clk_base,
@@ -2214,27 +2258,12 @@ struct clk *tegra_clk_register_plle_tegra114(const char *name,
{
struct tegra_clk_pll *pll;
struct clk *clk;
- u32 val, val_aux;
pll = _tegra_init_pll(clk_base, NULL, pll_params, lock);
if (IS_ERR(pll))
return ERR_CAST(pll);
- /* ensure parent is set to pll_re_vco */
-
- val = pll_readl_base(pll);
- val_aux = pll_readl(pll_params->aux_reg, pll);
-
- if (val & PLL_BASE_ENABLE) {
- if ((val_aux & PLLE_AUX_PLLRE_SEL) ||
- (val_aux & PLLE_AUX_PLLP_SEL))
- WARN(1, "pll_e enabled with unsupported parent %s\n",
- (val_aux & PLLE_AUX_PLLP_SEL) ? "pllp_out0" :
- "pll_re_vco");
- } else {
- val_aux &= ~(PLLE_AUX_PLLRE_SEL | PLLE_AUX_PLLP_SEL);
- pll_writel(val_aux, pll_params->aux_reg, pll);
- }
+ _clk_plle_tegra_init_parent(pll);
clk = _tegra_clk_register_pll(pll, name, parent_name, flags,
&tegra_clk_plle_tegra114_ops);
@@ -2276,6 +2305,7 @@ static const struct clk_ops tegra_clk_pllss_ops = {
.recalc_rate = clk_pll_recalc_rate,
.round_rate = clk_pll_ramp_round_rate,
.set_rate = clk_pllxc_set_rate,
+ .restore_context = tegra_clk_pll_restore_context,
};
struct clk *tegra_clk_register_pllss(const char *name, const char *parent_name,
@@ -2375,6 +2405,7 @@ struct clk *tegra_clk_register_pllre_tegra210(const char *name,
pll_params->vco_min = pll_params->adjust_vco(pll_params,
parent_rate);
+ pll_params->flags |= TEGRA_PLLRE;
pll = _tegra_init_pll(clk_base, pmc, pll_params, lock);
if (IS_ERR(pll))
return ERR_CAST(pll);
@@ -2520,11 +2551,19 @@ static void clk_plle_tegra210_disable(struct clk_hw *hw)
spin_unlock_irqrestore(pll->lock, flags);
}
+static void tegra_clk_plle_t210_restore_context(struct clk_hw *hw)
+{
+ struct tegra_clk_pll *pll = to_clk_pll(hw);
+
+ _clk_plle_tegra_init_parent(pll);
+}
+
static const struct clk_ops tegra_clk_plle_tegra210_ops = {
.is_enabled = clk_plle_tegra210_is_enabled,
.enable = clk_plle_tegra210_enable,
.disable = clk_plle_tegra210_disable,
.recalc_rate = clk_pll_recalc_rate,
+ .restore_context = tegra_clk_plle_t210_restore_context,
};
struct clk *tegra_clk_register_plle_tegra210(const char *name,
@@ -2535,27 +2574,12 @@ struct clk *tegra_clk_register_plle_tegra210(const char *name,
{
struct tegra_clk_pll *pll;
struct clk *clk;
- u32 val, val_aux;
pll = _tegra_init_pll(clk_base, NULL, pll_params, lock);
if (IS_ERR(pll))
return ERR_CAST(pll);
- /* ensure parent is set to pll_re_vco */
-
- val = pll_readl_base(pll);
- val_aux = pll_readl(pll_params->aux_reg, pll);
-
- if (val & PLLE_BASE_ENABLE) {
- if ((val_aux & PLLE_AUX_PLLRE_SEL) ||
- (val_aux & PLLE_AUX_PLLP_SEL))
- WARN(1, "pll_e enabled with unsupported parent %s\n",
- (val_aux & PLLE_AUX_PLLP_SEL) ? "pllp_out0" :
- "pll_re_vco");
- } else {
- val_aux &= ~(PLLE_AUX_PLLRE_SEL | PLLE_AUX_PLLP_SEL);
- pll_writel(val_aux, pll_params->aux_reg, pll);
- }
+ _clk_plle_tegra_init_parent(pll);
clk = _tegra_clk_register_pll(pll, name, parent_name, flags,
&tegra_clk_plle_tegra210_ops);
diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
index a464524fbc90..dc546292e030 100644
--- a/drivers/clk/tegra/clk.h
+++ b/drivers/clk/tegra/clk.h
@@ -233,6 +233,7 @@ struct tegra_clk_pll;
* TEGRA_PLLMB - PLLMB has should be treated similar to PLLM. This
* flag indicated that it is PLLMB.
* TEGRA_PLL_VCO_OUT - Used to indicate that the PLL has a VCO output
+ * TEGRA_PLLRE - Used to indicate that it is PLLRE
*/
struct tegra_clk_pll_params {
unsigned long input_min;
@@ -299,6 +300,7 @@ struct tegra_clk_pll_params {
#define TEGRA_MDIV_NEW BIT(11)
#define TEGRA_PLLMB BIT(12)
#define TEGRA_PLL_VCO_OUT BIT(13)
+#define TEGRA_PLLRE BIT(14)
/**
* struct tegra_clk_pll - Tegra PLL clock
--
2.7.4
^ permalink raw reply related
* [PATCH v8 16/21] soc/tegra: pmc: Add pmc wake support for tegra210
From: Sowjanya Komatineni @ 2019-08-08 23:46 UTC (permalink / raw)
To: thierry.reding, jonathanh, tglx, jason, marc.zyngier,
linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, skomatineni, linux-tegra, linux-kernel,
mperttunen, spatra, robh+dt, digetx, devicetree, rjw,
viresh.kumar, linux-pm
In-Reply-To: <1565308020-31952-1-git-send-email-skomatineni@nvidia.com>
This patch implements PMC wakeup sequence for Tegra210 and defines
common used RTC alarm wake event.
Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
drivers/soc/tegra/pmc.c | 98 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 98 insertions(+)
diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index 91c84d0e66ae..3aa71c28a10a 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -58,6 +58,11 @@
#define PMC_CNTRL_SYSCLK_POLARITY BIT(10) /* sys clk polarity */
#define PMC_CNTRL_MAIN_RST BIT(4)
+#define PMC_WAKE_MASK 0x0c
+#define PMC_WAKE_LEVEL 0x10
+#define PMC_WAKE_STATUS 0x14
+#define PMC_SW_WAKE_STATUS 0x18
+
#define DPD_SAMPLE 0x020
#define DPD_SAMPLE_ENABLE BIT(0)
#define DPD_SAMPLE_DISABLE (0 << 0)
@@ -87,6 +92,11 @@
#define PMC_SCRATCH41 0x140
+#define PMC_WAKE2_MASK 0x160
+#define PMC_WAKE2_LEVEL 0x164
+#define PMC_WAKE2_STATUS 0x168
+#define PMC_SW_WAKE2_STATUS 0x16c
+
#define PMC_SENSOR_CTRL 0x1b0
#define PMC_SENSOR_CTRL_SCRATCH_WRITE BIT(2)
#define PMC_SENSOR_CTRL_ENABLE_RST BIT(1)
@@ -1922,6 +1932,43 @@ static const struct irq_domain_ops tegra_pmc_irq_domain_ops = {
.alloc = tegra_pmc_irq_alloc,
};
+static int tegra210_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
+{
+ struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
+ unsigned int offset, bit;
+ u32 value;
+
+ if (data->hwirq == ULONG_MAX)
+ return 0;
+
+ offset = data->hwirq / 32;
+ bit = data->hwirq % 32;
+
+ /* clear wake status */
+ tegra_pmc_writel(pmc, 0, PMC_SW_WAKE_STATUS);
+ tegra_pmc_writel(pmc, 0, PMC_SW_WAKE2_STATUS);
+
+ tegra_pmc_writel(pmc, 0, PMC_WAKE_STATUS);
+ tegra_pmc_writel(pmc, 0, PMC_WAKE2_STATUS);
+
+ /* enable PMC wake */
+ if (data->hwirq >= 32)
+ offset = PMC_WAKE2_MASK;
+ else
+ offset = PMC_WAKE_MASK;
+
+ value = tegra_pmc_readl(pmc, offset);
+
+ if (on)
+ value |= 1 << bit;
+ else
+ value &= ~(1 << bit);
+
+ tegra_pmc_writel(pmc, value, offset);
+
+ return 0;
+}
+
static int tegra186_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
{
struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
@@ -1954,6 +2001,49 @@ static int tegra186_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
return 0;
}
+static int tegra210_pmc_irq_set_type(struct irq_data *data, unsigned int type)
+{
+ struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
+ unsigned int offset, bit;
+ u32 value;
+
+ if (data->hwirq == ULONG_MAX)
+ return 0;
+
+ offset = data->hwirq / 32;
+ bit = data->hwirq % 32;
+
+ if (data->hwirq >= 32)
+ offset = PMC_WAKE2_LEVEL;
+ else
+ offset = PMC_WAKE_LEVEL;
+
+ value = tegra_pmc_readl(pmc, offset);
+
+ switch (type) {
+ case IRQ_TYPE_EDGE_RISING:
+ case IRQ_TYPE_LEVEL_HIGH:
+ value |= 1 << bit;
+ break;
+
+ case IRQ_TYPE_EDGE_FALLING:
+ case IRQ_TYPE_LEVEL_LOW:
+ value &= ~(1 << bit);
+ break;
+
+ case IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING:
+ value ^= 1 << bit;
+ break;
+
+ default:
+ return -EINVAL;
+ }
+
+ tegra_pmc_writel(pmc, value, offset);
+
+ return 0;
+}
+
static int tegra186_pmc_irq_set_type(struct irq_data *data, unsigned int type)
{
struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
@@ -2540,6 +2630,10 @@ static const struct pinctrl_pin_desc tegra210_pin_descs[] = {
TEGRA210_IO_PAD_TABLE(TEGRA_IO_PIN_DESC)
};
+static const struct tegra_wake_event tegra210_wake_events[] = {
+ TEGRA_WAKE_IRQ("rtc", 16, 2),
+};
+
static const struct tegra_pmc_soc tegra210_pmc_soc = {
.num_powergates = ARRAY_SIZE(tegra210_powergates),
.powergates = tegra210_powergates,
@@ -2557,10 +2651,14 @@ static const struct tegra_pmc_soc tegra210_pmc_soc = {
.regs = &tegra20_pmc_regs,
.init = tegra20_pmc_init,
.setup_irq_polarity = tegra20_pmc_setup_irq_polarity,
+ .irq_set_wake = tegra210_pmc_irq_set_wake,
+ .irq_set_type = tegra210_pmc_irq_set_type,
.reset_sources = tegra210_reset_sources,
.num_reset_sources = ARRAY_SIZE(tegra210_reset_sources),
.reset_levels = NULL,
.num_reset_levels = 0,
+ .num_wake_events = ARRAY_SIZE(tegra210_wake_events),
+ .wake_events = tegra210_wake_events,
};
#define TEGRA186_IO_PAD_TABLE(_pad) \
--
2.7.4
^ permalink raw reply related
* [PATCH v8 15/21] soc/tegra: pmc: Allow to support more tegras wake
From: Sowjanya Komatineni @ 2019-08-08 23:46 UTC (permalink / raw)
To: thierry.reding, jonathanh, tglx, jason, marc.zyngier,
linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, skomatineni, linux-tegra, linux-kernel,
mperttunen, spatra, robh+dt, digetx, devicetree, rjw,
viresh.kumar, linux-pm
In-Reply-To: <1565308020-31952-1-git-send-email-skomatineni@nvidia.com>
This patch allows to create separate irq_set_wake and irq_set_type
implementations for different tegra designs PMC that has different
wake models which require difference wake registers and different
programming sequence.
AOWAKE model support is available for Tegra186 and Tegra194 only
and it resides within PMC and supports tiered wake architecture.
Tegra210 and prior tegra designs uses PMC directly to receive wake
events and coordinate the wake sequence.
Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
drivers/soc/tegra/pmc.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index 9f9c1c677cf4..91c84d0e66ae 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -226,6 +226,8 @@ struct tegra_pmc_soc {
void (*setup_irq_polarity)(struct tegra_pmc *pmc,
struct device_node *np,
bool invert);
+ int (*irq_set_wake)(struct irq_data *data, unsigned int on);
+ int (*irq_set_type)(struct irq_data *data, unsigned int type);
const char * const *reset_sources;
unsigned int num_reset_sources;
@@ -1920,7 +1922,7 @@ static const struct irq_domain_ops tegra_pmc_irq_domain_ops = {
.alloc = tegra_pmc_irq_alloc,
};
-static int tegra_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
+static int tegra186_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
{
struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
unsigned int offset, bit;
@@ -1952,7 +1954,7 @@ static int tegra_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
return 0;
}
-static int tegra_pmc_irq_set_type(struct irq_data *data, unsigned int type)
+static int tegra186_pmc_irq_set_type(struct irq_data *data, unsigned int type)
{
struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
u32 value;
@@ -2006,8 +2008,8 @@ static int tegra_pmc_irq_init(struct tegra_pmc *pmc)
pmc->irq.irq_unmask = irq_chip_unmask_parent;
pmc->irq.irq_eoi = irq_chip_eoi_parent;
pmc->irq.irq_set_affinity = irq_chip_set_affinity_parent;
- pmc->irq.irq_set_type = tegra_pmc_irq_set_type;
- pmc->irq.irq_set_wake = tegra_pmc_irq_set_wake;
+ pmc->irq.irq_set_type = pmc->soc->irq_set_type;
+ pmc->irq.irq_set_wake = pmc->soc->irq_set_wake;
pmc->domain = irq_domain_add_hierarchy(parent, 0, 96, pmc->dev->of_node,
&tegra_pmc_irq_domain_ops, pmc);
@@ -2680,6 +2682,8 @@ static const struct tegra_pmc_soc tegra186_pmc_soc = {
.regs = &tegra186_pmc_regs,
.init = NULL,
.setup_irq_polarity = tegra186_pmc_setup_irq_polarity,
+ .irq_set_wake = tegra186_pmc_irq_set_wake,
+ .irq_set_type = tegra186_pmc_irq_set_type,
.reset_sources = tegra186_reset_sources,
.num_reset_sources = ARRAY_SIZE(tegra186_reset_sources),
.reset_levels = tegra186_reset_levels,
--
2.7.4
^ permalink raw reply related
* [PATCH v8 17/21] arm64: tegra: Enable wake from deep sleep on RTC alarm
From: Sowjanya Komatineni @ 2019-08-08 23:46 UTC (permalink / raw)
To: thierry.reding, jonathanh, tglx, jason, marc.zyngier,
linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, skomatineni, linux-tegra, linux-kernel,
mperttunen, spatra, robh+dt, digetx, devicetree, rjw,
viresh.kumar, linux-pm
In-Reply-To: <1565308020-31952-1-git-send-email-skomatineni@nvidia.com>
This patch updates device tree for RTC and PMC to allow system wake
from deep sleep on RTC alarm.
Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
arch/arm64/boot/dts/nvidia/tegra210.dtsi | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
index 659753118e96..30a7c48385a2 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
@@ -768,7 +768,8 @@
rtc@7000e000 {
compatible = "nvidia,tegra210-rtc", "nvidia,tegra20-rtc";
reg = <0x0 0x7000e000 0x0 0x100>;
- interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>;
+ interrupts = <16 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-parent = <&pmc>;
clocks = <&tegra_car TEGRA210_CLK_RTC>;
clock-names = "rtc";
};
@@ -778,6 +779,8 @@
reg = <0x0 0x7000e400 0x0 0x400>;
clocks = <&tegra_car TEGRA210_CLK_PCLK>, <&clk32k_in>;
clock-names = "pclk", "clk32k_in";
+ #interrupt-cells = <2>;
+ interrupt-controller;
powergates {
pd_audio: aud {
--
2.7.4
^ permalink raw reply related
* [PATCH v8 14/21] clk: tegra210: Add suspend and resume support
From: Sowjanya Komatineni @ 2019-08-08 23:46 UTC (permalink / raw)
To: thierry.reding, jonathanh, tglx, jason, marc.zyngier,
linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, skomatineni, linux-tegra, linux-kernel,
mperttunen, spatra, robh+dt, digetx, devicetree, rjw,
viresh.kumar, linux-pm
In-Reply-To: <1565308020-31952-1-git-send-email-skomatineni@nvidia.com>
This patch adds support for clk: tegra210: suspend-resume.
All the CAR controller settings are lost on suspend when core
power goes off.
This patch has implementation for saving and restoring all PLLs
and clocks context during system suspend and resume to have the
clocks back to same state for normal operation.
Clock driver suspend and resume are registered as syscore_ops as clocks
restore need to happen before the other drivers resume to have all their
clocks back to the same state as before suspend.
Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
drivers/clk/tegra/clk-tegra210.c | 103 +++++++++++++++++++++++++++++++++++++--
drivers/clk/tegra/clk.c | 64 ++++++++++++++++++++++++
drivers/clk/tegra/clk.h | 3 ++
3 files changed, 166 insertions(+), 4 deletions(-)
diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c
index 998bf60b219a..8dd6f4f4debb 100644
--- a/drivers/clk/tegra/clk-tegra210.c
+++ b/drivers/clk/tegra/clk-tegra210.c
@@ -9,13 +9,13 @@
#include <linux/clkdev.h>
#include <linux/of.h>
#include <linux/of_address.h>
+#include <linux/syscore_ops.h>
#include <linux/delay.h>
#include <linux/export.h>
#include <linux/mutex.h>
#include <linux/clk/tegra.h>
#include <dt-bindings/clock/tegra210-car.h>
#include <dt-bindings/reset/tegra210-car.h>
-#include <linux/iopoll.h>
#include <linux/sizes.h>
#include <soc/tegra/pmc.h>
@@ -220,11 +220,15 @@
#define CLK_M_DIVISOR_SHIFT 2
#define CLK_M_DIVISOR_MASK 0x3
+#define CLK_MASK_ARM 0x44
+#define MISC_CLK_ENB 0x48
+
#define RST_DFLL_DVCO 0x2f4
#define DVFS_DFLL_RESET_SHIFT 0
#define CLK_RST_CONTROLLER_RST_DEV_Y_SET 0x2a8
#define CLK_RST_CONTROLLER_RST_DEV_Y_CLR 0x2ac
+#define CPU_SOFTRST_CTRL 0x380
#define LVL2_CLK_GATE_OVRA 0xf8
#define LVL2_CLK_GATE_OVRC 0x3a0
@@ -2825,6 +2829,7 @@ static int tegra210_enable_pllu(void)
struct tegra_clk_pll_freq_table *fentry;
struct tegra_clk_pll pllu;
u32 reg;
+ int ret;
for (fentry = pll_u_freq_table; fentry->input_rate; fentry++) {
if (fentry->input_rate == pll_ref_freq)
@@ -2853,9 +2858,14 @@ static int tegra210_enable_pllu(void)
reg |= PLL_ENABLE;
writel(reg, clk_base + PLLU_BASE);
- readl_relaxed_poll_timeout_atomic(clk_base + PLLU_BASE, reg,
- reg & PLL_BASE_LOCK, 2, 1000);
- if (!(reg & PLL_BASE_LOCK)) {
+ /*
+ * During clocks resume, same PLLU init and enable sequence get
+ * executed. So, readx_poll_timeout_atomic can't be used here as it
+ * uses ktime_get() and timekeeping resume doesn't happen by that
+ * time. So, using tegra210_wait_for_mask for PLL LOCK.
+ */
+ ret = tegra210_wait_for_mask(&pllu, PLLU_BASE, PLL_BASE_LOCK);
+ if (ret) {
pr_err("Timed out waiting for PLL_U to lock\n");
return -ETIMEDOUT;
}
@@ -3288,6 +3298,84 @@ static void tegra210_disable_cpu_clock(u32 cpu)
}
#ifdef CONFIG_PM_SLEEP
+/*
+ * This array lists mask values for each peripheral clk bank
+ * to mask out reserved bits during the clocks state restore
+ * on SC7 resume to prevent accidental writes to these reserved
+ * bits.
+ */
+static u32 periph_clk_rsvd_mask[TEGRA210_CAR_BANK_COUNT] = {
+ 0x23282006,
+ 0x782e0c18,
+ 0x0c012c05,
+ 0x003e7304,
+ 0x86c04800,
+ 0xc0199000,
+ 0x03e03800,
+};
+
+#define car_readl(_base, _off) readl_relaxed(clk_base + (_base) + ((_off) * 4))
+#define car_writel(_val, _base, _off) \
+ writel_relaxed(_val, clk_base + (_base) + ((_off) * 4))
+
+static u32 spare_reg_ctx, misc_clk_enb_ctx, clk_msk_arm_ctx;
+static u32 cpu_softrst_ctx[3];
+
+static int tegra210_clk_suspend(void)
+{
+ unsigned int i;
+
+ clk_save_context();
+
+ /*
+ * Save the bootloader configured clock registers SPARE_REG0,
+ * MISC_CLK_ENB, CLK_MASK_ARM, CPU_SOFTRST_CTRL.
+ */
+ spare_reg_ctx = readl_relaxed(clk_base + SPARE_REG0);
+ misc_clk_enb_ctx = readl_relaxed(clk_base + MISC_CLK_ENB);
+ clk_msk_arm_ctx = readl_relaxed(clk_base + CLK_MASK_ARM);
+
+ for (i = 0; i < ARRAY_SIZE(cpu_softrst_ctx); i++)
+ cpu_softrst_ctx[i] = car_readl(CPU_SOFTRST_CTRL, i);
+
+ tegra_clk_periph_suspend();
+ return 0;
+}
+
+static void tegra210_clk_resume(void)
+{
+ unsigned int i;
+
+ tegra_clk_osc_resume(clk_base);
+
+ /*
+ * Restore the bootloader configured clock registers SPARE_REG0,
+ * MISC_CLK_ENB, CLK_MASK_ARM, CPU_SOFTRST_CTRL from saved context.
+ */
+ writel_relaxed(spare_reg_ctx, clk_base + SPARE_REG0);
+ writel_relaxed(misc_clk_enb_ctx, clk_base + MISC_CLK_ENB);
+ writel_relaxed(clk_msk_arm_ctx, clk_base + CLK_MASK_ARM);
+
+ for (i = 0; i < ARRAY_SIZE(cpu_softrst_ctx); i++)
+ car_writel(cpu_softrst_ctx[i], CPU_SOFTRST_CTRL, i);
+
+ fence_udelay(5, clk_base);
+
+ /* enable all the clocks before changing the clock sources */
+ tegra_clk_periph_force_on(periph_clk_rsvd_mask);
+
+ /* wait for all writes to happen to have all the clocks enabled */
+ wmb();
+ fence_udelay(2, clk_base);
+
+ /* restore PLLs and all peripheral clock rates */
+ tegra210_init_pllu();
+ clk_restore_context();
+
+ /* restore all peripheral clocks enable and reset state */
+ tegra_clk_periph_resume();
+}
+
static void tegra210_cpu_clock_suspend(void)
{
/* switch coresite to clk_m, save off original source */
@@ -3303,6 +3391,11 @@ static void tegra210_cpu_clock_resume(void)
}
#endif
+static struct syscore_ops tegra_clk_syscore_ops = {
+ .suspend = tegra210_clk_suspend,
+ .resume = tegra210_clk_resume,
+};
+
static struct tegra_cpu_car_ops tegra210_cpu_car_ops = {
.wait_for_reset = tegra210_wait_cpu_in_reset,
.disable_clock = tegra210_disable_cpu_clock,
@@ -3587,5 +3680,7 @@ static void __init tegra210_clock_init(struct device_node *np)
tegra210_mbist_clk_init();
tegra_cpu_car_ops = &tegra210_cpu_car_ops;
+
+ register_syscore_ops(&tegra_clk_syscore_ops);
}
CLK_OF_DECLARE(tegra210, "nvidia,tegra210-car", tegra210_clock_init);
diff --git a/drivers/clk/tegra/clk.c b/drivers/clk/tegra/clk.c
index eb08047fd02f..368a576132f6 100644
--- a/drivers/clk/tegra/clk.c
+++ b/drivers/clk/tegra/clk.c
@@ -67,6 +67,7 @@ struct tegra_cpu_car_ops *tegra_cpu_car_ops = &dummy_car_ops;
int *periph_clk_enb_refcnt;
static int periph_banks;
+static u32 *periph_state_ctx;
static struct clk **clks;
static int clk_num;
static struct clk_onecell_data clk_data;
@@ -213,6 +214,61 @@ void tegra_clk_set_pllp_out_cpu(bool enable)
writel_relaxed(val, clk_base + CLK_OUT_ENB_Y);
}
+void tegra_clk_periph_force_on(u32 *clks_rsvd_mask)
+{
+ unsigned int i;
+
+ for (i = 0; i < periph_banks; i++)
+ writel_relaxed(~clks_rsvd_mask[i],
+ clk_base + periph_regs[i].enb_reg);
+}
+
+void tegra_clk_periph_suspend(void)
+{
+ unsigned int i, idx;
+
+ idx = 0;
+ for (i = 0; i < periph_banks; i++, idx++)
+ periph_state_ctx[idx] =
+ readl_relaxed(clk_base + periph_regs[i].enb_reg);
+
+ for (i = 0; i < periph_banks; i++, idx++)
+ periph_state_ctx[idx] =
+ readl_relaxed(clk_base + periph_regs[i].rst_reg);
+}
+
+void tegra_clk_periph_resume(void)
+{
+ unsigned int i, idx;
+
+ idx = 0;
+ for (i = 0; i < periph_banks; i++, idx++)
+ writel_relaxed(periph_state_ctx[idx],
+ clk_base + periph_regs[i].enb_reg);
+ /*
+ * All non-boot peripherals will be in reset state on resume.
+ * Wait for 5us of reset propagation delay before de-asserting
+ * the peripherals based on the saved context.
+ */
+ fence_udelay(5, clk_base);
+
+ for (i = 0; i < periph_banks; i++, idx++)
+ writel_relaxed(periph_state_ctx[idx],
+ clk_base + periph_regs[i].rst_reg);
+
+ fence_udelay(2, clk_base);
+}
+
+static int tegra_clk_periph_ctx_init(int banks)
+{
+ periph_state_ctx = kcalloc(2 * banks, sizeof(*periph_state_ctx),
+ GFP_KERNEL);
+ if (!periph_state_ctx)
+ return -ENOMEM;
+
+ return 0;
+}
+
struct clk ** __init tegra_clk_init(void __iomem *regs, int num, int banks)
{
clk_base = regs;
@@ -234,6 +290,14 @@ struct clk ** __init tegra_clk_init(void __iomem *regs, int num, int banks)
clk_num = num;
+ if (IS_ENABLED(CONFIG_PM_SLEEP)) {
+ if (tegra_clk_periph_ctx_init(banks)) {
+ kfree(periph_clk_enb_refcnt);
+ kfree(clks);
+ return NULL;
+ }
+ }
+
return clks;
}
diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
index 560e2bcb3d7d..9a17cad28d72 100644
--- a/drivers/clk/tegra/clk.h
+++ b/drivers/clk/tegra/clk.h
@@ -843,6 +843,9 @@ int div_frac_get(unsigned long rate, unsigned parent_rate, u8 width,
u8 frac_width, u8 flags);
void tegra_clk_osc_resume(void __iomem *clk_base);
void tegra_clk_set_pllp_out_cpu(bool enable);
+void tegra_clk_periph_force_on(u32 *clks_rsvd_mask);
+void tegra_clk_periph_suspend(void);
+void tegra_clk_periph_resume(void);
/* Combined read fence with delay */
--
2.7.4
^ permalink raw reply related
* [PATCH v8 20/21] arm64: dts: tegra210-p2180: Jetson TX1 SC7 timings
From: Sowjanya Komatineni @ 2019-08-08 23:46 UTC (permalink / raw)
To: thierry.reding, jonathanh, tglx, jason, marc.zyngier,
linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, skomatineni, linux-tegra, linux-kernel,
mperttunen, spatra, robh+dt, digetx, devicetree, rjw,
viresh.kumar, linux-pm
In-Reply-To: <1565308020-31952-1-git-send-email-skomatineni@nvidia.com>
This patch has Jetson TX1 platform specific SC7 timing configuration
in device tree.
Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi b/arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi
index 27723829d033..cb58f79deb48 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi
@@ -279,6 +279,13 @@
pmc@7000e400 {
nvidia,invert-interrupt;
+ nvidia,suspend-mode = <0>;
+ nvidia,cpu-pwr-good-time = <0>;
+ nvidia,cpu-pwr-off-time = <0>;
+ nvidia,core-pwr-good-time = <4587 3876>;
+ nvidia,core-pwr-off-time = <39065>;
+ nvidia,core-power-req-active-high;
+ nvidia,sys-clock-req-active-high;
};
/* eMMC */
--
2.7.4
^ permalink raw reply related
* [PATCH v8 19/21] soc/tegra: pmc: Configure deep sleep control settings
From: Sowjanya Komatineni @ 2019-08-08 23:46 UTC (permalink / raw)
To: thierry.reding, jonathanh, tglx, jason, marc.zyngier,
linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, skomatineni, linux-tegra, linux-kernel,
mperttunen, spatra, robh+dt, digetx, devicetree, rjw,
viresh.kumar, linux-pm
In-Reply-To: <1565308020-31952-1-git-send-email-skomatineni@nvidia.com>
Tegra210 and prior Tegra chips have deep sleep entry and wakeup related
timings which are platform specific that should be configured before
entering into deep sleep.
Below are the timing specific configurations for deep sleep entry and
wakeup.
- Core rail power-on stabilization timer
- OSC clock stabilization timer after SOC rail power is stabilized.
- Core power off time is the minimum wake delay to keep the system
in deep sleep state irrespective of any quick wake event.
These values depends on the discharge time of regulators and turn OFF
time of the PMIC to allow the complete system to finish entering into
deep sleep state.
These values vary based on the platform design and are specified
through the device tree.
This patch has implementation to configure these timings which are must
to have for proper deep sleep and wakeup operations.
Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
drivers/soc/tegra/pmc.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index e013ada7e4e9..9a78d8417367 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -88,6 +88,8 @@
#define PMC_CPUPWRGOOD_TIMER 0xc8
#define PMC_CPUPWROFF_TIMER 0xcc
+#define PMC_COREPWRGOOD_TIMER 0x3c
+#define PMC_COREPWROFF_TIMER 0xe0
#define PMC_PWR_DET_VALUE 0xe4
@@ -2277,7 +2279,7 @@ static const struct tegra_pmc_regs tegra20_pmc_regs = {
static void tegra20_pmc_init(struct tegra_pmc *pmc)
{
- u32 value;
+ u32 value, osc, pmu, off;
/* Always enable CPU power request */
value = tegra_pmc_readl(pmc, PMC_CNTRL);
@@ -2303,6 +2305,15 @@ static void tegra20_pmc_init(struct tegra_pmc *pmc)
value = tegra_pmc_readl(pmc, PMC_CNTRL);
value |= PMC_CNTRL_SYSCLK_OE;
tegra_pmc_writel(pmc, value, PMC_CNTRL);
+
+ osc = DIV_ROUND_UP(pmc->core_osc_time * 8192, 1000000);
+ pmu = DIV_ROUND_UP(pmc->core_pmu_time * 32768, 1000000);
+ off = DIV_ROUND_UP(pmc->core_off_time * 32768, 1000000);
+ if (osc && pmu)
+ tegra_pmc_writel(pmc, ((osc << 8) & 0xff00) | (pmu & 0xff),
+ PMC_COREPWRGOOD_TIMER);
+ if (off)
+ tegra_pmc_writel(pmc, off, PMC_COREPWROFF_TIMER);
}
static void tegra20_pmc_setup_irq_polarity(struct tegra_pmc *pmc,
--
2.7.4
^ permalink raw reply related
* [PATCH v8 18/21] soc/tegra: pmc: Configure core power request polarity
From: Sowjanya Komatineni @ 2019-08-08 23:46 UTC (permalink / raw)
To: thierry.reding, jonathanh, tglx, jason, marc.zyngier,
linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, skomatineni, linux-tegra, linux-kernel,
mperttunen, spatra, robh+dt, digetx, devicetree, rjw,
viresh.kumar, linux-pm
In-Reply-To: <1565308020-31952-1-git-send-email-skomatineni@nvidia.com>
This patch configures polarity of the core power request signal
in PMC control register based on the device tree property.
PMC asserts and de-asserts power request signal based on it polarity
when it need to power-up and power-down the core rail during SC7.
Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
drivers/soc/tegra/pmc.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index 3aa71c28a10a..e013ada7e4e9 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -56,6 +56,7 @@
#define PMC_CNTRL_SIDE_EFFECT_LP0 BIT(14) /* LP0 when CPU pwr gated */
#define PMC_CNTRL_SYSCLK_OE BIT(11) /* system clock enable */
#define PMC_CNTRL_SYSCLK_POLARITY BIT(10) /* sys clk polarity */
+#define PMC_CNTRL_PWRREQ_POLARITY BIT(8)
#define PMC_CNTRL_MAIN_RST BIT(4)
#define PMC_WAKE_MASK 0x0c
@@ -2290,6 +2291,11 @@ static void tegra20_pmc_init(struct tegra_pmc *pmc)
else
value |= PMC_CNTRL_SYSCLK_POLARITY;
+ if (pmc->corereq_high)
+ value &= ~PMC_CNTRL_PWRREQ_POLARITY;
+ else
+ value |= PMC_CNTRL_PWRREQ_POLARITY;
+
/* configure the output polarity while the request is tristated */
tegra_pmc_writel(pmc, value, PMC_CNTRL);
--
2.7.4
^ permalink raw reply related
* [PATCH v8 21/21] arm64: dts: tegra210-p3450: Jetson Nano SC7 timings
From: Sowjanya Komatineni @ 2019-08-08 23:47 UTC (permalink / raw)
To: thierry.reding, jonathanh, tglx, jason, marc.zyngier,
linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, skomatineni, linux-tegra, linux-kernel,
mperttunen, spatra, robh+dt, digetx, devicetree, rjw,
viresh.kumar, linux-pm
In-Reply-To: <1565308020-31952-1-git-send-email-skomatineni@nvidia.com>
This patch adds Jetson Nano platform specific SC7 timing configuration
in the device tree.
Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
arch/arm64/boot/dts/nvidia/tegra210-p3450-0000.dts | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm64/boot/dts/nvidia/tegra210-p3450-0000.dts b/arch/arm64/boot/dts/nvidia/tegra210-p3450-0000.dts
index 9d17ec707bce..b525e69c172a 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210-p3450-0000.dts
+++ b/arch/arm64/boot/dts/nvidia/tegra210-p3450-0000.dts
@@ -382,6 +382,13 @@
pmc@7000e400 {
nvidia,invert-interrupt;
+ nvidia,suspend-mode = <0>;
+ nvidia,cpu-pwr-good-time = <0>;
+ nvidia,cpu-pwr-off-time = <0>;
+ nvidia,core-pwr-good-time = <4587 3876>;
+ nvidia,core-pwr-off-time = <39065>;
+ nvidia,core-power-req-active-high;
+ nvidia,sys-clock-req-active-high;
};
hda@70030000 {
--
2.7.4
^ permalink raw reply related
* [PATCH v8 02/21] pinctrl: tegra: Add write barrier after all pinctrl register writes
From: Sowjanya Komatineni @ 2019-08-08 23:46 UTC (permalink / raw)
To: thierry.reding, jonathanh, tglx, jason, marc.zyngier,
linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, skomatineni, linux-tegra, linux-kernel,
mperttunen, spatra, robh+dt, digetx, devicetree, rjw,
viresh.kumar, linux-pm
In-Reply-To: <1565308020-31952-1-git-send-email-skomatineni@nvidia.com>
This patch adds write barrier after all pinctrl register writes
during resume to make sure all pinctrl changes are complete.
Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
drivers/pinctrl/tegra/pinctrl-tegra.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c b/drivers/pinctrl/tegra/pinctrl-tegra.c
index 982ee634b3b1..f49fe29fb6df 100644
--- a/drivers/pinctrl/tegra/pinctrl-tegra.c
+++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
@@ -677,6 +677,8 @@ static int tegra_pinctrl_resume(struct device *dev)
writel_relaxed(*backup_regs++, regs++);
}
+ /* make sure all the pinmux register writes are complete */
+ wmb();
return 0;
}
--
2.7.4
^ permalink raw reply related
* Applied "spi: Rename of_spi_register_master() function" to the spi tree
From: Mark Brown @ 2019-08-08 20:33 UTC (permalink / raw)
To: Linus Walleij; +Cc: Bartosz Golaszewski, linux-gpio, linux-spi, Mark Brown
In-Reply-To: <20190808150321.23319-1-linus.walleij@linaro.org>
The patch
spi: Rename of_spi_register_master() function
has been applied to the spi tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-5.4
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
From 43004f31ebf2705905460a6d9a77da4182170c38 Mon Sep 17 00:00:00 2001
From: Linus Walleij <linus.walleij@linaro.org>
Date: Thu, 8 Aug 2019 17:03:21 +0200
Subject: [PATCH] spi: Rename of_spi_register_master() function
Rename this function to of_spi_get_gpio_numbers() as this
is what the function does, it does not register a master,
it is called in the path of registering a master so the
name is logical in a convoluted way, but it is better to
follow Rusty Russell's ABI level no 7:
"The obvious use is (probably) the correct one"
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Link: https://lore.kernel.org/r/20190808150321.23319-1-linus.walleij@linaro.org
Signed-off-by: Mark Brown <broonie@kernel.org>
---
drivers/spi/spi.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 8e83c9567353..aef55acb5ccd 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -2235,7 +2235,7 @@ struct spi_controller *__spi_alloc_controller(struct device *dev,
EXPORT_SYMBOL_GPL(__spi_alloc_controller);
#ifdef CONFIG_OF
-static int of_spi_register_master(struct spi_controller *ctlr)
+static int of_spi_get_gpio_numbers(struct spi_controller *ctlr)
{
int nb, i, *cs;
struct device_node *np = ctlr->dev.of_node;
@@ -2268,7 +2268,7 @@ static int of_spi_register_master(struct spi_controller *ctlr)
return 0;
}
#else
-static int of_spi_register_master(struct spi_controller *ctlr)
+static int of_spi_get_gpio_numbers(struct spi_controller *ctlr)
{
return 0;
}
@@ -2455,7 +2455,7 @@ int spi_register_controller(struct spi_controller *ctlr)
ctlr->mode_bits |= SPI_CS_HIGH;
} else {
/* Legacy code path for GPIOs from DT */
- status = of_spi_register_master(ctlr);
+ status = of_spi_get_gpio_numbers(ctlr);
if (status)
return status;
}
--
2.20.1
^ permalink raw reply related
* Re: [libgpiod] [PATCH 5/5] bindings: cxx: Workaround --success run
From: Alexander Stein @ 2019-08-08 18:41 UTC (permalink / raw)
To: Bartosz Golaszewski; +Cc: open list:GPIO SUBSYSTEM
In-Reply-To: <CAMRc=McmKs=5ToTRLgQ_f30DRtqq-57OZRw-EnL1gm_M1kqUXQ@mail.gmail.com>
On Thursday, August 8, 2019, 5:27:14 PM CEST Bartosz Golaszewski wrote:
> śr., 7 sie 2019 o 21:51 Alexander Stein <alexander.stein@mailbox.org>
> napisał(a):
> >
> > If run with --success, all expressions are evaluated and printed out.
> > But REQUIRE_FALSE(chip) tries to iterate over the chip resulting in this
> > backtrace:
> > #0 gpiod_chip_num_lines (chip=chip@entry=0x0) at ../../lib/core.c:235
> > #1 gpiod_line_iter_new (chip=0x0) at ../../lib/iter.c:140
> > #2 gpiod::(anonymous namespace)::make_line_iter (chip=0x0) at ../../../bindings/cxx/iter.cpp:29
> > #3 gpiod::line_iter::line_iter (this=0x7fffffffd690, owner=...) at ../../../bindings/cxx/iter.cpp:109
> > #4 Catch::rangeToString<gpiod::chip> (range=...) at /usr/include/catch2/catch.hpp:1959
> > [...]
> >
> > Workaround by forcing catch2 to call gpiod::chip::operator bool().
> >
> > Signed-off-by: Alexander Stein <alexander.stein@mailbox.org>
> > ---
> > This actually looks like a flaw in the binding itself that the
> > gpiod::line_iter can't cope with an empty gpiod::chip.
> >
>
> Do you want to submit a patch that fixes that? Otherwise I can fix it.
> I think that simply throwing an exception on empty chip is enough,
> right?
Reading that backtrace today, the actual problem is gpiod_chip_num_lines deferencing the nullptr.
There are 2 possibilities:
* if gpiod_chip is NULL in gpiod_line_iter_new(), return NULLL iter as well (which will raise an exception on line iter.cpp:31)
* return an iter with num_lines = 0
Can't rate the 2nd one if this will raise other problems.
Best regards,
Alexander
^ permalink raw reply
* Re: [libgpiod] [PATCH 4/5] bindings: cxx: Fix compile errors
From: Bartosz Golaszewski @ 2019-08-08 15:37 UTC (permalink / raw)
To: Alexander Stein; +Cc: open list:GPIO SUBSYSTEM
In-Reply-To: <CAMRc=MdnbDR2fK1qrqwapTXGm2OMdKjmSpicEWg93XB=ORoJrA@mail.gmail.com>
czw., 8 sie 2019 o 17:25 Bartosz Golaszewski <brgl@bgdev.pl> napisał(a):
>
> śr., 7 sie 2019 o 21:51 Alexander Stein <alexander.stein@mailbox.org>
> napisał(a):
> >
> > This fixes the following compile errors:
> > tests-event.cpp:152:3: error: cannot declare reference to
> > 'class std::system_error&', which is not a typedef or a template type
> > argument
> > 152 | REQUIRE_THROWS_AS(line.event_get_fd(), ::std::system_error&);
> >
>
> Hi Alexander, thanks for working on this!
>
> I'm getting the following warning when I don't use the reference:
>
> In file included from tests-line.cpp:8:
> tests-line.cpp: In function ‘void ____C_A_T_C_H____T_E_S_T____24()’:
> tests-line.cpp:254:45: warning: catching polymorphic type ‘class
> std::system_error’ by value [-Wcatch-value=]
> REQUIRE_THROWS_AS(line.get_value(), ::std::system_error);
> ^~~~~~~~~~~~
> I'm also not getting any build errors with my current next or master
> branch. My gcc is:
>
> gcc (Debian 8.3.0-6) 8.3.0
> Copyright (C) 2018 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions. There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>
> Bart
Nevermind that - I was using Debian's packaged version which happens
to be quite old. The current version actually does fail to build. I'll
test and pick up your patches tomorrow.
Bart
>
> > Signed-off-by: Alexander Stein <alexander.stein@mailbox.org>
> > ---
> > bindings/cxx/tests/tests-chip.cpp | 8 ++++----
> > bindings/cxx/tests/tests-event.cpp | 4 ++--
> > bindings/cxx/tests/tests-line.cpp | 16 ++++++++--------
> > 3 files changed, 14 insertions(+), 14 deletions(-)
> >
> > diff --git a/bindings/cxx/tests/tests-chip.cpp b/bindings/cxx/tests/tests-chip.cpp
> > index 11c2d4c..c9eb8e5 100644
> > --- a/bindings/cxx/tests/tests-chip.cpp
> > +++ b/bindings/cxx/tests/tests-chip.cpp
> > @@ -107,7 +107,7 @@ TEST_CASE("Uninitialized GPIO chip behaves correctly", "[chip]")
> >
> > SECTION("using uninitialized chip throws logic_error")
> > {
> > - REQUIRE_THROWS_AS(chip.name(), ::std::logic_error&);
> > + REQUIRE_THROWS_AS(chip.name(), ::std::logic_error);
> > }
> > }
> >
> > @@ -139,7 +139,7 @@ TEST_CASE("GPIO chip can be opened with the open() method with implicit lookup",
> >
> > TEST_CASE("Trying to open a nonexistent chip throws system_error", "[chip]")
> > {
> > - REQUIRE_THROWS_AS(::gpiod::chip("nonexistent-chip"), ::std::system_error&);
> > + REQUIRE_THROWS_AS(::gpiod::chip("nonexistent-chip"), ::std::system_error);
> > }
> >
> > TEST_CASE("Chip object can be reset", "[chip]")
> > @@ -244,12 +244,12 @@ TEST_CASE("Errors occurring when retrieving lines are correctly reported", "[chi
> >
> > SECTION("invalid offset (single line)")
> > {
> > - REQUIRE_THROWS_AS(chip.get_line(9), ::std::out_of_range&);
> > + REQUIRE_THROWS_AS(chip.get_line(9), ::std::out_of_range);
> > }
> >
> > SECTION("invalid offset (multiple lines)")
> > {
> > - REQUIRE_THROWS_AS(chip.get_lines({ 1, 19, 4, 7 }), ::std::out_of_range&);
> > + REQUIRE_THROWS_AS(chip.get_lines({ 1, 19, 4, 7 }), ::std::out_of_range);
> > }
> >
> > SECTION("line not found by name")
> > diff --git a/bindings/cxx/tests/tests-event.cpp b/bindings/cxx/tests/tests-event.cpp
> > index b34347f..b41cf7e 100644
> > --- a/bindings/cxx/tests/tests-event.cpp
> > +++ b/bindings/cxx/tests/tests-event.cpp
> > @@ -149,7 +149,7 @@ TEST_CASE("It's possible to retrieve the event file descriptor", "[event][line]"
> >
> > SECTION("error if not requested")
> > {
> > - REQUIRE_THROWS_AS(line.event_get_fd(), ::std::system_error&);
> > + REQUIRE_THROWS_AS(line.event_get_fd(), ::std::system_error);
> > }
> >
> > SECTION("error if requested for values")
> > @@ -157,7 +157,7 @@ TEST_CASE("It's possible to retrieve the event file descriptor", "[event][line]"
> > config.request_type = ::gpiod::line_request::DIRECTION_INPUT;
> >
> > line.request(config);
> > - REQUIRE_THROWS_AS(line.event_get_fd(), ::std::system_error&);
> > + REQUIRE_THROWS_AS(line.event_get_fd(), ::std::system_error);
> > }
> > }
> >
> > diff --git a/bindings/cxx/tests/tests-line.cpp b/bindings/cxx/tests/tests-line.cpp
> > index e827e60..08ff1e8 100644
> > --- a/bindings/cxx/tests/tests-line.cpp
> > +++ b/bindings/cxx/tests/tests-line.cpp
> > @@ -122,7 +122,7 @@ TEST_CASE("Line bulk object works correctly", "[line][bulk]")
> > {
> > auto lines = chip.get_all_lines();
> >
> > - REQUIRE_THROWS_AS(lines.get(11), ::std::out_of_range&);
> > + REQUIRE_THROWS_AS(lines.get(11), ::std::out_of_range);
> > }
> > }
> >
> > @@ -242,7 +242,7 @@ TEST_CASE("Exported line can be released", "[line]")
> > line.release();
> >
> > REQUIRE_FALSE(line.is_requested());
> > - REQUIRE_THROWS_AS(line.get_value(), ::std::system_error&);
> > + REQUIRE_THROWS_AS(line.get_value(), ::std::system_error);
> > }
> >
> > TEST_CASE("Uninitialized GPIO line behaves correctly", "[line]")
> > @@ -256,7 +256,7 @@ TEST_CASE("Uninitialized GPIO line behaves correctly", "[line]")
> >
> > SECTION("using uninitialized line throws logic_error")
> > {
> > - REQUIRE_THROWS_AS(line.name(), ::std::logic_error&);
> > + REQUIRE_THROWS_AS(line.name(), ::std::logic_error);
> > }
> > }
> >
> > @@ -271,7 +271,7 @@ TEST_CASE("Uninitialized GPIO line_bulk behaves correctly", "[line][bulk]")
> >
> > SECTION("using uninitialized line_bulk throws logic_error")
> > {
> > - REQUIRE_THROWS_AS(bulk.get(0), ::std::logic_error&);
> > + REQUIRE_THROWS_AS(bulk.get(0), ::std::logic_error);
> > }
> > }
> >
> > @@ -289,7 +289,7 @@ TEST_CASE("Cannot request the same line twice", "[line]")
> > auto line = chip.get_line(3);
> >
> > REQUIRE_NOTHROW(line.request(config));
> > - REQUIRE_THROWS_AS(line.request(config), ::std::system_error&);
> > + REQUIRE_THROWS_AS(line.request(config), ::std::system_error);
> > }
> >
> > SECTION("request the same line twice in line_bulk")
> > @@ -300,7 +300,7 @@ TEST_CASE("Cannot request the same line twice", "[line]")
> > */
> > auto lines = chip.get_lines({ 2, 3, 4, 4 });
> >
> > - REQUIRE_THROWS_AS(lines.request(config), ::std::system_error&);
> > + REQUIRE_THROWS_AS(lines.request(config), ::std::system_error);
> > }
> > }
> >
> > @@ -312,12 +312,12 @@ TEST_CASE("Cannot get/set values of unrequested lines", "[line]")
> >
> > SECTION("get value")
> > {
> > - REQUIRE_THROWS_AS(line.get_value(), ::std::system_error&);
> > + REQUIRE_THROWS_AS(line.get_value(), ::std::system_error);
> > }
> >
> > SECTION("set value")
> > {
> > - REQUIRE_THROWS_AS(line.set_value(1), ::std::system_error&);
> > + REQUIRE_THROWS_AS(line.set_value(1), ::std::system_error);
> > }
> > }
> >
> > --
> > 2.22.0
> >
^ permalink raw reply
* Re: [libgpiod] [PATCH 5/5] bindings: cxx: Workaround --success run
From: Bartosz Golaszewski @ 2019-08-08 15:27 UTC (permalink / raw)
To: Alexander Stein; +Cc: open list:GPIO SUBSYSTEM
In-Reply-To: <20190807195132.7538-5-alexander.stein@mailbox.org>
śr., 7 sie 2019 o 21:51 Alexander Stein <alexander.stein@mailbox.org>
napisał(a):
>
> If run with --success, all expressions are evaluated and printed out.
> But REQUIRE_FALSE(chip) tries to iterate over the chip resulting in this
> backtrace:
> #0 gpiod_chip_num_lines (chip=chip@entry=0x0) at ../../lib/core.c:235
> #1 gpiod_line_iter_new (chip=0x0) at ../../lib/iter.c:140
> #2 gpiod::(anonymous namespace)::make_line_iter (chip=0x0) at ../../../bindings/cxx/iter.cpp:29
> #3 gpiod::line_iter::line_iter (this=0x7fffffffd690, owner=...) at ../../../bindings/cxx/iter.cpp:109
> #4 Catch::rangeToString<gpiod::chip> (range=...) at /usr/include/catch2/catch.hpp:1959
> [...]
>
> Workaround by forcing catch2 to call gpiod::chip::operator bool().
>
> Signed-off-by: Alexander Stein <alexander.stein@mailbox.org>
> ---
> This actually looks like a flaw in the binding itself that the
> gpiod::line_iter can't cope with an empty gpiod::chip.
>
Do you want to submit a patch that fixes that? Otherwise I can fix it.
I think that simply throwing an exception on empty chip is enough,
right?
Bart
> bindings/cxx/tests/tests-chip.cpp | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/bindings/cxx/tests/tests-chip.cpp b/bindings/cxx/tests/tests-chip.cpp
> index c9eb8e5..1c69872 100644
> --- a/bindings/cxx/tests/tests-chip.cpp
> +++ b/bindings/cxx/tests/tests-chip.cpp
> @@ -70,7 +70,7 @@ TEST_CASE("GPIO chip can be opened with the open() method in different modes", "
> mockup::probe_guard mockup_chips({ 8, 8, 8 });
> ::gpiod::chip chip;
>
> - REQUIRE_FALSE(chip);
> + REQUIRE_FALSE(!!chip);
>
> SECTION("open by name")
> {
> @@ -102,7 +102,7 @@ TEST_CASE("Uninitialized GPIO chip behaves correctly", "[chip]")
>
> SECTION("uninitialized chip is 'false'")
> {
> - REQUIRE_FALSE(chip);
> + REQUIRE_FALSE(!!chip);
> }
>
> SECTION("using uninitialized chip throws logic_error")
> @@ -149,7 +149,7 @@ TEST_CASE("Chip object can be reset", "[chip]")
> ::gpiod::chip chip(mockup::instance().chip_name(0));
> REQUIRE(chip);
> chip.reset();
> - REQUIRE_FALSE(chip);
> + REQUIRE_FALSE(!!chip);
> }
>
> TEST_CASE("Chip info can be correctly retrieved", "[chip]")
> --
> 2.22.0
>
^ permalink raw reply
* Re: [libgpiod] [PATCH 4/5] bindings: cxx: Fix compile errors
From: Bartosz Golaszewski @ 2019-08-08 15:25 UTC (permalink / raw)
To: Alexander Stein; +Cc: open list:GPIO SUBSYSTEM
In-Reply-To: <20190807195132.7538-4-alexander.stein@mailbox.org>
śr., 7 sie 2019 o 21:51 Alexander Stein <alexander.stein@mailbox.org>
napisał(a):
>
> This fixes the following compile errors:
> tests-event.cpp:152:3: error: cannot declare reference to
> 'class std::system_error&', which is not a typedef or a template type
> argument
> 152 | REQUIRE_THROWS_AS(line.event_get_fd(), ::std::system_error&);
>
Hi Alexander, thanks for working on this!
I'm getting the following warning when I don't use the reference:
In file included from tests-line.cpp:8:
tests-line.cpp: In function ‘void ____C_A_T_C_H____T_E_S_T____24()’:
tests-line.cpp:254:45: warning: catching polymorphic type ‘class
std::system_error’ by value [-Wcatch-value=]
REQUIRE_THROWS_AS(line.get_value(), ::std::system_error);
^~~~~~~~~~~~
I'm also not getting any build errors with my current next or master
branch. My gcc is:
gcc (Debian 8.3.0-6) 8.3.0
Copyright (C) 2018 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
Bart
> Signed-off-by: Alexander Stein <alexander.stein@mailbox.org>
> ---
> bindings/cxx/tests/tests-chip.cpp | 8 ++++----
> bindings/cxx/tests/tests-event.cpp | 4 ++--
> bindings/cxx/tests/tests-line.cpp | 16 ++++++++--------
> 3 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/bindings/cxx/tests/tests-chip.cpp b/bindings/cxx/tests/tests-chip.cpp
> index 11c2d4c..c9eb8e5 100644
> --- a/bindings/cxx/tests/tests-chip.cpp
> +++ b/bindings/cxx/tests/tests-chip.cpp
> @@ -107,7 +107,7 @@ TEST_CASE("Uninitialized GPIO chip behaves correctly", "[chip]")
>
> SECTION("using uninitialized chip throws logic_error")
> {
> - REQUIRE_THROWS_AS(chip.name(), ::std::logic_error&);
> + REQUIRE_THROWS_AS(chip.name(), ::std::logic_error);
> }
> }
>
> @@ -139,7 +139,7 @@ TEST_CASE("GPIO chip can be opened with the open() method with implicit lookup",
>
> TEST_CASE("Trying to open a nonexistent chip throws system_error", "[chip]")
> {
> - REQUIRE_THROWS_AS(::gpiod::chip("nonexistent-chip"), ::std::system_error&);
> + REQUIRE_THROWS_AS(::gpiod::chip("nonexistent-chip"), ::std::system_error);
> }
>
> TEST_CASE("Chip object can be reset", "[chip]")
> @@ -244,12 +244,12 @@ TEST_CASE("Errors occurring when retrieving lines are correctly reported", "[chi
>
> SECTION("invalid offset (single line)")
> {
> - REQUIRE_THROWS_AS(chip.get_line(9), ::std::out_of_range&);
> + REQUIRE_THROWS_AS(chip.get_line(9), ::std::out_of_range);
> }
>
> SECTION("invalid offset (multiple lines)")
> {
> - REQUIRE_THROWS_AS(chip.get_lines({ 1, 19, 4, 7 }), ::std::out_of_range&);
> + REQUIRE_THROWS_AS(chip.get_lines({ 1, 19, 4, 7 }), ::std::out_of_range);
> }
>
> SECTION("line not found by name")
> diff --git a/bindings/cxx/tests/tests-event.cpp b/bindings/cxx/tests/tests-event.cpp
> index b34347f..b41cf7e 100644
> --- a/bindings/cxx/tests/tests-event.cpp
> +++ b/bindings/cxx/tests/tests-event.cpp
> @@ -149,7 +149,7 @@ TEST_CASE("It's possible to retrieve the event file descriptor", "[event][line]"
>
> SECTION("error if not requested")
> {
> - REQUIRE_THROWS_AS(line.event_get_fd(), ::std::system_error&);
> + REQUIRE_THROWS_AS(line.event_get_fd(), ::std::system_error);
> }
>
> SECTION("error if requested for values")
> @@ -157,7 +157,7 @@ TEST_CASE("It's possible to retrieve the event file descriptor", "[event][line]"
> config.request_type = ::gpiod::line_request::DIRECTION_INPUT;
>
> line.request(config);
> - REQUIRE_THROWS_AS(line.event_get_fd(), ::std::system_error&);
> + REQUIRE_THROWS_AS(line.event_get_fd(), ::std::system_error);
> }
> }
>
> diff --git a/bindings/cxx/tests/tests-line.cpp b/bindings/cxx/tests/tests-line.cpp
> index e827e60..08ff1e8 100644
> --- a/bindings/cxx/tests/tests-line.cpp
> +++ b/bindings/cxx/tests/tests-line.cpp
> @@ -122,7 +122,7 @@ TEST_CASE("Line bulk object works correctly", "[line][bulk]")
> {
> auto lines = chip.get_all_lines();
>
> - REQUIRE_THROWS_AS(lines.get(11), ::std::out_of_range&);
> + REQUIRE_THROWS_AS(lines.get(11), ::std::out_of_range);
> }
> }
>
> @@ -242,7 +242,7 @@ TEST_CASE("Exported line can be released", "[line]")
> line.release();
>
> REQUIRE_FALSE(line.is_requested());
> - REQUIRE_THROWS_AS(line.get_value(), ::std::system_error&);
> + REQUIRE_THROWS_AS(line.get_value(), ::std::system_error);
> }
>
> TEST_CASE("Uninitialized GPIO line behaves correctly", "[line]")
> @@ -256,7 +256,7 @@ TEST_CASE("Uninitialized GPIO line behaves correctly", "[line]")
>
> SECTION("using uninitialized line throws logic_error")
> {
> - REQUIRE_THROWS_AS(line.name(), ::std::logic_error&);
> + REQUIRE_THROWS_AS(line.name(), ::std::logic_error);
> }
> }
>
> @@ -271,7 +271,7 @@ TEST_CASE("Uninitialized GPIO line_bulk behaves correctly", "[line][bulk]")
>
> SECTION("using uninitialized line_bulk throws logic_error")
> {
> - REQUIRE_THROWS_AS(bulk.get(0), ::std::logic_error&);
> + REQUIRE_THROWS_AS(bulk.get(0), ::std::logic_error);
> }
> }
>
> @@ -289,7 +289,7 @@ TEST_CASE("Cannot request the same line twice", "[line]")
> auto line = chip.get_line(3);
>
> REQUIRE_NOTHROW(line.request(config));
> - REQUIRE_THROWS_AS(line.request(config), ::std::system_error&);
> + REQUIRE_THROWS_AS(line.request(config), ::std::system_error);
> }
>
> SECTION("request the same line twice in line_bulk")
> @@ -300,7 +300,7 @@ TEST_CASE("Cannot request the same line twice", "[line]")
> */
> auto lines = chip.get_lines({ 2, 3, 4, 4 });
>
> - REQUIRE_THROWS_AS(lines.request(config), ::std::system_error&);
> + REQUIRE_THROWS_AS(lines.request(config), ::std::system_error);
> }
> }
>
> @@ -312,12 +312,12 @@ TEST_CASE("Cannot get/set values of unrequested lines", "[line]")
>
> SECTION("get value")
> {
> - REQUIRE_THROWS_AS(line.get_value(), ::std::system_error&);
> + REQUIRE_THROWS_AS(line.get_value(), ::std::system_error);
> }
>
> SECTION("set value")
> {
> - REQUIRE_THROWS_AS(line.set_value(1), ::std::system_error&);
> + REQUIRE_THROWS_AS(line.set_value(1), ::std::system_error);
> }
> }
>
> --
> 2.22.0
>
^ permalink raw reply
* [PATCH] spi: Rename of_spi_register_master() function
From: Linus Walleij @ 2019-08-08 15:03 UTC (permalink / raw)
To: Mark Brown, linux-spi; +Cc: linux-gpio, Bartosz Golaszewski, Linus Walleij
Rename this function to of_spi_get_gpio_numbers() as this
is what the function does, it does not register a master,
it is called in the path of registering a master so the
name is logical in a convoluted way, but it is better to
follow Rusty Russell's ABI level no 7:
"The obvious use is (probably) the correct one"
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/spi/spi.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 75ac046cae52..79d7526e1337 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -2236,7 +2236,7 @@ struct spi_controller *__spi_alloc_controller(struct device *dev,
EXPORT_SYMBOL_GPL(__spi_alloc_controller);
#ifdef CONFIG_OF
-static int of_spi_register_master(struct spi_controller *ctlr)
+static int of_spi_get_gpio_numbers(struct spi_controller *ctlr)
{
int nb, i, *cs;
struct device_node *np = ctlr->dev.of_node;
@@ -2269,7 +2269,7 @@ static int of_spi_register_master(struct spi_controller *ctlr)
return 0;
}
#else
-static int of_spi_register_master(struct spi_controller *ctlr)
+static int of_spi_get_gpio_numbers(struct spi_controller *ctlr)
{
return 0;
}
@@ -2456,7 +2456,7 @@ int spi_register_controller(struct spi_controller *ctlr)
ctlr->mode_bits |= SPI_CS_HIGH;
} else {
/* Legacy code path for GPIOs from DT */
- status = of_spi_register_master(ctlr);
+ status = of_spi_get_gpio_numbers(ctlr);
if (status)
return status;
}
--
2.21.0
^ permalink raw reply related
* Re: [PATCH] pinctrl: freescale: imx: Add of_node_put() before return
From: Philipp Zabel @ 2019-08-08 14:56 UTC (permalink / raw)
To: Nishka Dasgupta, aisheng.dong, festevam, shawnguo, stefan, kernel,
linus.walleij, s.hauer, linux-imx, linux-gpio, linux-arm-kernel
In-Reply-To: <20190808074720.15754-1-nishkadg.linux@gmail.com>
On Thu, 2019-08-08 at 13:17 +0530, Nishka Dasgupta wrote:
> Each iteration of for_each_child_of_node() puts the previous node;
> however, in the case of a return from the middle of the loop, there is no
> put, thus causing a memory leak. Hence put of_node_put() statements as
> required before two mid-loop return statements.
> Issue found with Coccinelle.
>
> Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
> ---
> drivers/pinctrl/freescale/pinctrl-imx.c | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c b/drivers/pinctrl/freescale/pinctrl-imx.c
> index 83ff9532bae6..9f42036c5fbb 100644
> --- a/drivers/pinctrl/freescale/pinctrl-imx.c
> +++ b/drivers/pinctrl/freescale/pinctrl-imx.c
> @@ -672,8 +672,10 @@ static int imx_pinctrl_parse_functions(struct device_node *np,
>
> grp = devm_kzalloc(ipctl->dev, sizeof(struct group_desc),
> GFP_KERNEL);
This looks to me like it could just allocate an array of struct
group_desc upfront, just like the group_names array. Same for the
functions in imx_pinctrl_probe_dt(). Not an issue with this patch
though.
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
regards
Philipp
> - if (!grp)
> + if (!grp) {
> + of_node_put(child);
> return -ENOMEM;
> + }
>
> mutex_lock(&ipctl->mutex);
> radix_tree_insert(&pctl->pin_group_tree,
> @@ -697,12 +699,17 @@ static bool imx_pinctrl_dt_is_flat_functions(struct device_node *np)
> struct device_node *pinctrl_np;
>
> for_each_child_of_node(np, function_np) {
> - if (of_property_read_bool(function_np, "fsl,pins"))
> + if (of_property_read_bool(function_np, "fsl,pins")) {
> + of_node_put(function_np);
> return true;
> + }
>
> for_each_child_of_node(function_np, pinctrl_np) {
> - if (of_property_read_bool(pinctrl_np, "fsl,pins"))
> + if (of_property_read_bool(pinctrl_np, "fsl,pins")) {
> + of_node_put(pinctrl_np);
> + of_node_put(function_np);
> return false;
> + }
> }
> }
>
^ permalink raw reply
* Re: [PATCH 1/2] gpiolib: Add for_each_gpio_suffix() helper
From: Stefan Roese @ 2019-08-08 14:13 UTC (permalink / raw)
To: Andy Shevchenko
Cc: linux-serial, linux-gpio, Geert Uytterhoeven, Pavel Machek,
Linus Walleij, Greg Kroah-Hartman
In-Reply-To: <20190808134402.GX30120@smile.fi.intel.com>
Hi Andy,
On 08.08.19 15:44, Andy Shevchenko wrote:
> On Thu, Aug 08, 2019 at 03:25:42PM +0200, Stefan Roese wrote:
>> Add a helper macro to enable the interation over all supported GPIO
>> suffixes (currently "gpios" & "gpio"). This will be used by the serial
>> mctrl code to check, if a GPIO property exists before requesting it.
>
> Thanks! My comments below.
>
>> +#define for_each_gpio_suffix(idx, suffix) \
>> + for (idx = 0; \
>> + idx < ARRAY_SIZE(gpio_suffixes) && \
>> + (suffix = gpio_suffixes[idx]) != NULL; \
>> + idx++)
>
> I think we can use comma here, like
>
> for (idx = 0; idx < ARRAY_SIZE(...), suffix = ...; idx++;)
>
> however, this needs to be checked, b/c I think the last operation makes a
> return code, and probably we have to assign suffix first.
Yes, this seems to be the case (assign suffix first). But in this
case, the assignment is done *before* checking if the index is still
valid (in the array). In this case "suffix = gpio_suffices[2]". Or am
I missing something?
> (and perhaps switch to one letter for idx makes it fit one line)
Yes.
Thanks,
Stefan
^ permalink raw reply
* Re: [PATCH 2/2] serial: mctrl_gpio: Support all GPIO suffixes (gpios vs gpio)
From: Stefan Roese @ 2019-08-08 13:59 UTC (permalink / raw)
To: Andy Shevchenko
Cc: linux-serial, linux-gpio, Geert Uytterhoeven, Pavel Machek,
Linus Walleij, Greg Kroah-Hartman
In-Reply-To: <20190808134859.GY30120@smile.fi.intel.com>
Hi Andy,
On 08.08.19 15:48, Andy Shevchenko wrote:
> On Thu, Aug 08, 2019 at 03:25:43PM +0200, Stefan Roese wrote:
>> This patch fixes a backward compatibility issue, when boards use the
>> old style GPIO suffix "-gpio" instead of the new "-gpios". This
>> potential problem has been introduced by commit d99482673f95 ("serial:
>> mctrl_gpio: Check if GPIO property exisits before requesting it").
>>
>> This patch now fixes this issue by iterating over all supported GPIO
>> suffixes by using the newly introduced for_each_gpio_suffix() helper.
>>
>> Also, the string buffer is now allocated on the stack to avoid the
>> problem of allocation in a loop and its potential failure.
>
>> for (i = 0; i < UART_GPIO_MAX; i++) {
>> enum gpiod_flags flags;
>> - char *gpio_str;
>> + const char *suffix;
>> + char gpio_str[32]; /* 32 is max size of property name */
>
> Hmm... don't we have some define for the maximum length of property?
I've come up with this assumption from this code (identical comment):
https://elixir.bootlin.com/linux/latest/source/drivers/gpio/gpiolib-of.c#L293
(and other places in drivers/gpio/*)
> Or maybe we can still continue using kasprintf() approach?
Frankly, I was feeling a bit uncomfortable with this memory allocation
in a loop. And Pavel also commented on this:
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg2066286.html
So I would really prefer to move this buffer to the stack instead.
>> bool present;
>> + int k;
>> +
>> + /*
>> + * Check if GPIO property exists and continue if not. Iterate
>> + * over all supported GPIO suffixes (foo-gpios vs. foo-gpio).
>> + */
>> + for_each_gpio_suffix(k, suffix) {
>> + snprintf(gpio_str, sizeof(gpio_str), "%s-%s",
>> + mctrl_gpios_desc[i].name, suffix);
>> +
>> + present = device_property_present(dev, gpio_str);
>> + if (present)
>> + break;
>> + }
>>
>> - /* Check if GPIO property exists and continue if not */
>> - gpio_str = kasprintf(GFP_KERNEL, "%s-gpios",
>> - mctrl_gpios_desc[i].name);
>> - if (!gpio_str)
>> - continue;
>> -
>
>> - present = device_property_present(dev, gpio_str);
>
> Because there is no more explicit assignment of present outside of the loop,
> the compiler may warn about uninitialized variable in use...
>
>> - kfree(gpio_str);
>
>> if (!present)
>
> ...here.
My compiler does not warn (MIPS GCC 8.1) but I see your point. I'll add
an initialization in the next version.
Thanks,
Stefan
^ permalink raw reply
* Re: [PATCH 2/2] serial: mctrl_gpio: Support all GPIO suffixes (gpios vs gpio)
From: Andy Shevchenko @ 2019-08-08 13:48 UTC (permalink / raw)
To: Stefan Roese
Cc: linux-serial, linux-gpio, Geert Uytterhoeven, Pavel Machek,
Linus Walleij, Greg Kroah-Hartman
In-Reply-To: <20190808132543.26274-2-sr@denx.de>
On Thu, Aug 08, 2019 at 03:25:43PM +0200, Stefan Roese wrote:
> This patch fixes a backward compatibility issue, when boards use the
> old style GPIO suffix "-gpio" instead of the new "-gpios". This
> potential problem has been introduced by commit d99482673f95 ("serial:
> mctrl_gpio: Check if GPIO property exisits before requesting it").
>
> This patch now fixes this issue by iterating over all supported GPIO
> suffixes by using the newly introduced for_each_gpio_suffix() helper.
>
> Also, the string buffer is now allocated on the stack to avoid the
> problem of allocation in a loop and its potential failure.
> for (i = 0; i < UART_GPIO_MAX; i++) {
> enum gpiod_flags flags;
> - char *gpio_str;
> + const char *suffix;
> + char gpio_str[32]; /* 32 is max size of property name */
Hmm... don't we have some define for the maximum length of property?
Or maybe we can still continue using kasprintf() approach?
> bool present;
> + int k;
> +
> + /*
> + * Check if GPIO property exists and continue if not. Iterate
> + * over all supported GPIO suffixes (foo-gpios vs. foo-gpio).
> + */
> + for_each_gpio_suffix(k, suffix) {
> + snprintf(gpio_str, sizeof(gpio_str), "%s-%s",
> + mctrl_gpios_desc[i].name, suffix);
> +
> + present = device_property_present(dev, gpio_str);
> + if (present)
> + break;
> + }
>
> - /* Check if GPIO property exists and continue if not */
> - gpio_str = kasprintf(GFP_KERNEL, "%s-gpios",
> - mctrl_gpios_desc[i].name);
> - if (!gpio_str)
> - continue;
> -
> - present = device_property_present(dev, gpio_str);
Because there is no more explicit assignment of present outside of the loop,
the compiler may warn about uninitialized variable in use...
> - kfree(gpio_str);
> if (!present)
...here.
> continue;
--
With Best Regards,
Andy Shevchenko
^ 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