[PATCH v3 0/4] Restore ULPI USB on Tegra20

[PATCH v3 0/4] Restore ULPI USB on Tegra20

[Dmitry Osipenko]

Hello,

This series of patches fixes ULPI USB on Tegra20. The original problem
was reported by Marcel Ziswiler, he found that "ulpi-link" clock was
incorrectly set to CDEV2 instead of PLL_P_OUT4. Marcel made a patch
that changed the "ulpi-link" clock to PLL_P_OUT4 and that fixed issue
with the USB for the devices that have CDEV2 being enabled by bootloader.
The patch got into the kernel and later Marc Dietrich found that USB
stopped working on the "paz00" Tegra20 board. After a bit of discussion
was revealed that PLL_P_OUT4 is the parent clock of the CDEV2 and clock
driver was setting CDEV2's parent incorrectly. The parent clock is actually
determined by the pinmuxing config of CDEV2 pingroup. This patchset fixes
the parent of CDEV2 clock by making Tegra's pinctrl driver a clock provider,
providing CDEV1/2 clock muxes (thanks to Peter De Schrijver for the
suggestion), and then setting these clock muxes as parents for the CDEV1/2
clocks. In the end Marcel's CDEV2->PLL_P_OUT4 change is reverted since CDEV2
(aka MCLK2) is the actual clock source for "ulpi-link".

Changelog:

v3:
	- Use clk DT ID's instead of comparing clk names and make
	  custom of_src_onecell_get specific to Tegra20 clk provider
	  in the "Add quirk for getting CDEV1/2 clocks on Tegra20" patch
	  as was suggested by Peter De Schrijver for v2.

v2:
	- Added new patch "Add quirk for getting CDEV1/2 clocks", assuring
	  that clk user won't get CDEV1/2 clocks until parent clk muxes
	  are available, i.e. resolves potential issue with CDEV-user driver
	  vs pinctrl driver probe order.

	- Factored out "pinctrl" patch from the patchset as was requested by
	  Linus Walleij.

	- Addressed v1 review comments: fixed swapped DEV1/2 clk div bits,
	  made DEV1/2 divs read-only, etc minor changes.

Dmitry Osipenko (4):
  clk: tegra20: Add DEV1/DEV2 OSC dividers
  clk: tegra20: Correct parents of CDEV1/2 clocks
  clk: tegra: Add quirk for getting CDEV1/2 clocks on Tegra20
  ARM: dts: tegra20: Revert "Fix ULPI regression on Tegra20"

 arch/arm/boot/dts/tegra20.dtsi   |  2 +-
 drivers/clk/tegra/clk-tegra114.c |  2 +-
 drivers/clk/tegra/clk-tegra124.c |  2 +-
 drivers/clk/tegra/clk-tegra20.c  | 52 +++++++++++++++++++++++++++++---
 drivers/clk/tegra/clk-tegra210.c |  2 +-
 drivers/clk/tegra/clk-tegra30.c  |  2 +-
 drivers/clk/tegra/clk.c          |  5 +--
 drivers/clk/tegra/clk.h          |  2 +-
 8 files changed, 56 insertions(+), 13 deletions(-)

-- 
2.17.0

[PATCH v3 1/4] clk: tegra20: Add DEV1/DEV2 OSC dividers

[Dmitry Osipenko]

CDEV1/CDEV2 clocks could have corresponding oscillator clock divider as
a parent. Add these dividers in order to be able to provide that parent
option.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
Reviewed-by: Marcel Ziswiler <marcel@ziswiler.com>
Tested-by: Marcel Ziswiler <marcel@ziswiler.com>
Tested-by: Marc Dietrich <marvin24@gmx.de>
Acked-by: Peter De Schrijver <pdeschrijver@nvidia.com>
Acked-by: Stephen Boyd <sboyd@kernel.org>
---
 drivers/clk/tegra/clk-tegra20.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-tegra20.c
index 0ee56dd04cec..ad5a7b5e3a39 100644
--- a/drivers/clk/tegra/clk-tegra20.c
+++ b/drivers/clk/tegra/clk-tegra20.c
@@ -26,6 +26,8 @@
 #include "clk.h"
 #include "clk-id.h"
 
+#define MISC_CLK_ENB 0x48
+
 #define OSC_CTRL 0x50
 #define OSC_CTRL_OSC_FREQ_MASK (3<<30)
 #define OSC_CTRL_OSC_FREQ_13MHZ (0<<30)
@@ -831,6 +833,18 @@ static void __init tegra20_periph_clk_init(void)
 				    periph_clk_enb_refcnt);
 	clks[TEGRA20_CLK_PEX] = clk;
 
+	/* dev1 OSC divider */
+	clk_register_divider(NULL, "dev1_osc_div", "clk_m",
+			     0, clk_base + MISC_CLK_ENB, 22, 2,
+			     CLK_DIVIDER_POWER_OF_TWO | CLK_DIVIDER_READ_ONLY,
+			     NULL);
+
+	/* dev2 OSC divider */
+	clk_register_divider(NULL, "dev2_osc_div", "clk_m",
+			     0, clk_base + MISC_CLK_ENB, 20, 2,
+			     CLK_DIVIDER_POWER_OF_TWO | CLK_DIVIDER_READ_ONLY,
+			     NULL);
+
 	/* cdev1 */
 	clk = clk_register_fixed_rate(NULL, "cdev1_fixed", NULL, 0, 26000000);
 	clk = tegra_clk_register_periph_gate("cdev1", "cdev1_fixed", 0,
-- 
2.17.0

[PATCH v3] pinctrl: tegra20: Provide CDEV1/2 clock muxes

[Dmitry Osipenko]

Muxing of pins MCLK1/2 determine the muxing of the corresponding clocks.
Make pinctrl driver to provide clock muxes for the CDEV1/2 pingroups, so
that main clk-controller driver could get an actual parent clock for the
CDEV1/2 clocks.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
Reviewed-by: Marcel Ziswiler <marcel@ziswiler.com>
Tested-by: Marcel Ziswiler <marcel@ziswiler.com>
Tested-by: Marc Dietrich <marvin24@gmx.de>
Acked-by: Peter De Schrijver <pdeschrijver@nvidia.com>
---

Changelog:

v3:
	- No change.

v2:
	- See changelog of "Restore ULPI USB on Tegra20" v2 series.

 drivers/pinctrl/tegra/pinctrl-tegra.c   | 11 ---------
 drivers/pinctrl/tegra/pinctrl-tegra.h   | 11 +++++++++
 drivers/pinctrl/tegra/pinctrl-tegra20.c | 30 ++++++++++++++++++++++++-
 3 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c b/drivers/pinctrl/tegra/pinctrl-tegra.c
index 72c718e66ebb..49c7c1499bc3 100644
--- a/drivers/pinctrl/tegra/pinctrl-tegra.c
+++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
@@ -33,17 +33,6 @@
 #include "../pinctrl-utils.h"
 #include "pinctrl-tegra.h"
 
-struct tegra_pmx {
-	struct device *dev;
-	struct pinctrl_dev *pctl;
-
-	const struct tegra_pinctrl_soc_data *soc;
-	const char **group_pins;
-
-	int nbanks;
-	void __iomem **regs;
-};
-
 static inline u32 pmx_readl(struct tegra_pmx *pmx, u32 bank, u32 reg)
 {
 	return readl(pmx->regs[bank] + reg);
diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.h b/drivers/pinctrl/tegra/pinctrl-tegra.h
index 33b17cb1471e..aa33c20766c4 100644
--- a/drivers/pinctrl/tegra/pinctrl-tegra.h
+++ b/drivers/pinctrl/tegra/pinctrl-tegra.h
@@ -16,6 +16,17 @@
 #ifndef __PINMUX_TEGRA_H__
 #define __PINMUX_TEGRA_H__
 
+struct tegra_pmx {
+	struct device *dev;
+	struct pinctrl_dev *pctl;
+
+	const struct tegra_pinctrl_soc_data *soc;
+	const char **group_pins;
+
+	int nbanks;
+	void __iomem **regs;
+};
+
 enum tegra_pinconf_param {
 	/* argument: tegra_pinconf_pull */
 	TEGRA_PINCONF_PARAM_PULL,
diff --git a/drivers/pinctrl/tegra/pinctrl-tegra20.c b/drivers/pinctrl/tegra/pinctrl-tegra20.c
index 7e38ee9bae78..b6dd939d32cc 100644
--- a/drivers/pinctrl/tegra/pinctrl-tegra20.c
+++ b/drivers/pinctrl/tegra/pinctrl-tegra20.c
@@ -19,6 +19,7 @@
  * more details.
  */
 
+#include <linux/clk-provider.h>
 #include <linux/init.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
@@ -2231,9 +2232,36 @@ static const struct tegra_pinctrl_soc_data tegra20_pinctrl = {
 	.drvtype_in_mux = false,
 };
 
+static const char *cdev1_parents[] = {
+	"dev1_osc_div", "pll_a_out0", "pll_m_out1", "audio",
+};
+
+static const char *cdev2_parents[] = {
+	"dev2_osc_div", "hclk", "pclk", "pll_p_out4",
+};
+
+static void tegra20_pinctrl_register_clock_muxes(struct platform_device *pdev)
+{
+	struct tegra_pmx *pmx = platform_get_drvdata(pdev);
+
+	clk_register_mux(NULL, "cdev1_mux", cdev1_parents, 4, 0,
+			 pmx->regs[1] + 0x8, 2, 2, CLK_MUX_READ_ONLY, NULL);
+
+	clk_register_mux(NULL, "cdev2_mux", cdev2_parents, 4, 0,
+			 pmx->regs[1] + 0x8, 4, 2, CLK_MUX_READ_ONLY, NULL);
+}
+
 static int tegra20_pinctrl_probe(struct platform_device *pdev)
 {
-	return tegra_pinctrl_probe(pdev, &tegra20_pinctrl);
+	int err;
+
+	err = tegra_pinctrl_probe(pdev, &tegra20_pinctrl);
+	if (err)
+		return err;
+
+	tegra20_pinctrl_register_clock_muxes(pdev);
+
+	return 0;
 }
 
 static const struct of_device_id tegra20_pinctrl_of_match[] = {
-- 
2.17.0

[PATCH v3 2/4] clk: tegra20: Correct parents of CDEV1/2 clocks

[Dmitry Osipenko]

Parents of CDEV1/2 clocks are determined by muxing of the corresponding
pins. Pinctrl driver now provides the CDEV1/2 clock muxes and hence
CDEV1/2 clocks could have correct parents. Set CDEV1/2 parents to the
corresponding muxes to fix the parents.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
Reviewed-by: Marcel Ziswiler <marcel@ziswiler.com>
Tested-by: Marcel Ziswiler <marcel@ziswiler.com>
Tested-by: Marc Dietrich <marvin24@gmx.de>
Acked-by: Peter De Schrijver <pdeschrijver@nvidia.com>
Acked-by: Stephen Boyd <sboyd@kernel.org>
---
 drivers/clk/tegra/clk-tegra20.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-tegra20.c
index ad5a7b5e3a39..636500a98561 100644
--- a/drivers/clk/tegra/clk-tegra20.c
+++ b/drivers/clk/tegra/clk-tegra20.c
@@ -846,14 +846,12 @@ static void __init tegra20_periph_clk_init(void)
 			     NULL);
 
 	/* cdev1 */
-	clk = clk_register_fixed_rate(NULL, "cdev1_fixed", NULL, 0, 26000000);
-	clk = tegra_clk_register_periph_gate("cdev1", "cdev1_fixed", 0,
+	clk = tegra_clk_register_periph_gate("cdev1", "cdev1_mux", 0,
 				    clk_base, 0, 94, periph_clk_enb_refcnt);
 	clks[TEGRA20_CLK_CDEV1] = clk;
 
 	/* cdev2 */
-	clk = clk_register_fixed_rate(NULL, "cdev2_fixed", NULL, 0, 26000000);
-	clk = tegra_clk_register_periph_gate("cdev2", "cdev2_fixed", 0,
+	clk = tegra_clk_register_periph_gate("cdev2", "cdev2_mux", 0,
 				    clk_base, 0, 93, periph_clk_enb_refcnt);
 	clks[TEGRA20_CLK_CDEV2] = clk;
 
-- 
2.17.0

[PATCH v3 3/4] clk: tegra: Add quirk for getting CDEV1/2 clocks on Tegra20

[Dmitry Osipenko]

CDEV1 and CDEV2 clocks are a bit special case, their parent clock is
created by the pinctrl driver. It should be possible for clk user to
request these clocks before pinctrl driver got probed and hence user will
get an orphaned clock. That might be undesirable because user may expect
parent clock to be enabled by the child, so let's return -EPROBE_DEFER
till parent clock appears.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/clk/tegra/clk-tegra114.c |  2 +-
 drivers/clk/tegra/clk-tegra124.c |  2 +-
 drivers/clk/tegra/clk-tegra20.c  | 32 +++++++++++++++++++++++++++++++-
 drivers/clk/tegra/clk-tegra210.c |  2 +-
 drivers/clk/tegra/clk-tegra30.c  |  2 +-
 drivers/clk/tegra/clk.c          |  5 +++--
 drivers/clk/tegra/clk.h          |  2 +-
 7 files changed, 39 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/tegra/clk-tegra114.c b/drivers/clk/tegra/clk-tegra114.c
index 5d5a22d529f5..1824f014202b 100644
--- a/drivers/clk/tegra/clk-tegra114.c
+++ b/drivers/clk/tegra/clk-tegra114.c
@@ -1367,7 +1367,7 @@ static void __init tegra114_clock_init(struct device_node *np)
 	tegra_super_clk_gen4_init(clk_base, pmc_base, tegra114_clks,
 					&pll_x_params);
 
-	tegra_add_of_provider(np);
+	tegra_add_of_provider(np, of_clk_src_onecell_get);
 	tegra_register_devclks(devclks, ARRAY_SIZE(devclks));
 
 	tegra_clk_apply_init_table = tegra114_clock_apply_init_table;
diff --git a/drivers/clk/tegra/clk-tegra124.c b/drivers/clk/tegra/clk-tegra124.c
index 50088e976611..0c69c7970950 100644
--- a/drivers/clk/tegra/clk-tegra124.c
+++ b/drivers/clk/tegra/clk-tegra124.c
@@ -1479,7 +1479,7 @@ static void __init tegra124_132_clock_init_post(struct device_node *np)
 				  &pll_x_params);
 	tegra_init_special_resets(1, tegra124_reset_assert,
 				  tegra124_reset_deassert);
-	tegra_add_of_provider(np);
+	tegra_add_of_provider(np, of_clk_src_onecell_get);
 
 	clks[TEGRA124_CLK_EMC] = tegra_clk_register_emc(clk_base, np,
 							&emc_lock);
diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-tegra20.c
index 636500a98561..cc857d4d4a86 100644
--- a/drivers/clk/tegra/clk-tegra20.c
+++ b/drivers/clk/tegra/clk-tegra20.c
@@ -1089,6 +1089,36 @@ static const struct of_device_id pmc_match[] __initconst = {
 	{ },
 };
 
+static struct clk *tegra20_clk_src_onecell_get(struct of_phandle_args *clkspec,
+					       void *data)
+{
+	struct clk_hw *parent_hw;
+	struct clk_hw *hw;
+	struct clk *clk;
+
+	clk = of_clk_src_onecell_get(clkspec, data);
+	if (IS_ERR(clk))
+		return clk;
+
+	/*
+	 * Tegra20 CDEV1 and CDEV2 clocks are a bit special case, their parent
+	 * clock is created by the pinctrl driver. It is possible for clk user
+	 * to request these clocks before pinctrl driver got probed and hence
+	 * user will get an orphaned clock. That might be undesirable because
+	 * user may expect parent clock to be enabled by the child.
+	 */
+	if (clkspec->args[0] == TEGRA20_CLK_CDEV1 ||
+	    clkspec->args[0] == TEGRA20_CLK_CDEV2) {
+		hw = __clk_get_hw(clk);
+
+		parent_hw = clk_hw_get_parent(hw);
+		if (!parent_hw)
+			return ERR_PTR(-EPROBE_DEFER);
+	}
+
+	return clk;
+}
+
 static void __init tegra20_clock_init(struct device_node *np)
 {
 	struct device_node *node;
@@ -1127,7 +1157,7 @@ static void __init tegra20_clock_init(struct device_node *np)
 
 	tegra_init_dup_clks(tegra_clk_duplicates, clks, TEGRA20_CLK_CLK_MAX);
 
-	tegra_add_of_provider(np);
+	tegra_add_of_provider(np, tegra20_clk_src_onecell_get);
 	tegra_register_devclks(devclks, ARRAY_SIZE(devclks));
 
 	tegra_clk_apply_init_table = tegra20_clock_apply_init_table;
diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c
index 9fb5d51ccce4..5435d01c636a 100644
--- a/drivers/clk/tegra/clk-tegra210.c
+++ b/drivers/clk/tegra/clk-tegra210.c
@@ -3567,7 +3567,7 @@ static void __init tegra210_clock_init(struct device_node *np)
 	tegra_init_special_resets(2, tegra210_reset_assert,
 				  tegra210_reset_deassert);
 
-	tegra_add_of_provider(np);
+	tegra_add_of_provider(np, of_clk_src_onecell_get);
 	tegra_register_devclks(devclks, ARRAY_SIZE(devclks));
 
 	tegra210_mbist_clk_init();
diff --git a/drivers/clk/tegra/clk-tegra30.c b/drivers/clk/tegra/clk-tegra30.c
index b316dfb6f6c7..acfe661b2ae7 100644
--- a/drivers/clk/tegra/clk-tegra30.c
+++ b/drivers/clk/tegra/clk-tegra30.c
@@ -1349,7 +1349,7 @@ static void __init tegra30_clock_init(struct device_node *np)
 
 	tegra_init_dup_clks(tegra_clk_duplicates, clks, TEGRA30_CLK_CLK_MAX);
 
-	tegra_add_of_provider(np);
+	tegra_add_of_provider(np, of_clk_src_onecell_get);
 	tegra_register_devclks(devclks, ARRAY_SIZE(devclks));
 
 	tegra_clk_apply_init_table = tegra30_clock_apply_init_table;
diff --git a/drivers/clk/tegra/clk.c b/drivers/clk/tegra/clk.c
index ba923f0d5953..593d76a114f9 100644
--- a/drivers/clk/tegra/clk.c
+++ b/drivers/clk/tegra/clk.c
@@ -298,7 +298,8 @@ static struct reset_controller_dev rst_ctlr = {
 	.of_reset_n_cells = 1,
 };
 
-void __init tegra_add_of_provider(struct device_node *np)
+void __init tegra_add_of_provider(struct device_node *np,
+				  void *clk_src_onecell_get)
 {
 	int i;
 
@@ -314,7 +315,7 @@ void __init tegra_add_of_provider(struct device_node *np)
 
 	clk_data.clks = clks;
 	clk_data.clk_num = clk_num;
-	of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);
+	of_clk_add_provider(np, clk_src_onecell_get, &clk_data);
 
 	rst_ctlr.of_node = np;
 	rst_ctlr.nr_resets = periph_banks * 32 + num_special_reset;
diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
index ba7e20e6a82b..e1f88463b600 100644
--- a/drivers/clk/tegra/clk.h
+++ b/drivers/clk/tegra/clk.h
@@ -763,7 +763,7 @@ struct clk **tegra_clk_init(void __iomem *clk_base, int num, int periph_banks);
 
 struct clk **tegra_lookup_dt_id(int clk_id, struct tegra_clk *tegra_clk);
 
-void tegra_add_of_provider(struct device_node *np);
+void tegra_add_of_provider(struct device_node *np, void *clk_src_onecell_get);
 void tegra_register_devclks(struct tegra_devclk *dev_clks, int num);
 
 void tegra_audio_clk_init(void __iomem *clk_base,
-- 
2.17.0

[PATCH v3 4/4] ARM: dts: tegra20: Revert "Fix ULPI regression on Tegra20"

[Dmitry Osipenko]

Commit 4c9a27a6c66d ("ARM: tegra: Fix ULPI regression on Tegra20") changed
"ulpi-link" clock from CDEV2 to PLL_P_OUT4. Turned out that PLL_P_OUT4 is
the parent of CDEV2 clock and original clock setup of "ulpi-link" was
correct. The reverted patch was fixing USB for one board and broke the
other, now Tegra's clk driver correctly sets parent for the CDEV2 clock
and hence patch could be reverted safely, restoring USB for all of the
boards.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
Reviewed-by: Marcel Ziswiler <marcel@ziswiler.com>
Tested-by: Marcel Ziswiler <marcel@ziswiler.com>
Tested-by: Marc Dietrich <marvin24@gmx.de>
---
 arch/arm/boot/dts/tegra20.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
index 0a7136462a1a..983dd5c14794 100644
--- a/arch/arm/boot/dts/tegra20.dtsi
+++ b/arch/arm/boot/dts/tegra20.dtsi
@@ -741,7 +741,7 @@
 		phy_type = "ulpi";
 		clocks = <&tegra_car TEGRA20_CLK_USB2>,
 			 <&tegra_car TEGRA20_CLK_PLL_U>,
-			 <&tegra_car TEGRA20_CLK_PLL_P_OUT4>;
+			 <&tegra_car TEGRA20_CLK_CDEV2>;
 		clock-names = "reg", "pll_u", "ulpi-link";
 		resets = <&tegra_car 58>, <&tegra_car 22>;
 		reset-names = "usb", "utmi-pads";
-- 
2.17.0

Re: [PATCH v3 3/4] clk: tegra: Add quirk for getting CDEV1/2 clocks on Tegra20

[Peter De Schrijver]

On Tue, May 08, 2018 at 07:26:06PM +0300, Dmitry Osipenko wrote:
> CDEV1 and CDEV2 clocks are a bit special case, their parent clock is
> created by the pinctrl driver. It should be possible for clk user to
> request these clocks before pinctrl driver got probed and hence user will
> get an orphaned clock. That might be undesirable because user may expect
> parent clock to be enabled by the child, so let's return -EPROBE_DEFER
> till parent clock appears.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>

Acked-by: Peter De Schrijver <pdeschrijver@nvidia.com>                                                                                                     


> ---
>  drivers/clk/tegra/clk-tegra114.c |  2 +-
>  drivers/clk/tegra/clk-tegra124.c |  2 +-
>  drivers/clk/tegra/clk-tegra20.c  | 32 +++++++++++++++++++++++++++++++-
>  drivers/clk/tegra/clk-tegra210.c |  2 +-
>  drivers/clk/tegra/clk-tegra30.c  |  2 +-
>  drivers/clk/tegra/clk.c          |  5 +++--
>  drivers/clk/tegra/clk.h          |  2 +-
>  7 files changed, 39 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/clk/tegra/clk-tegra114.c b/drivers/clk/tegra/clk-tegra114.c
> index 5d5a22d529f5..1824f014202b 100644
> --- a/drivers/clk/tegra/clk-tegra114.c
> +++ b/drivers/clk/tegra/clk-tegra114.c
> @@ -1367,7 +1367,7 @@ static void __init tegra114_clock_init(struct device_node *np)
>  	tegra_super_clk_gen4_init(clk_base, pmc_base, tegra114_clks,
>  					&pll_x_params);
>  
> -	tegra_add_of_provider(np);
> +	tegra_add_of_provider(np, of_clk_src_onecell_get);
>  	tegra_register_devclks(devclks, ARRAY_SIZE(devclks));
>  
>  	tegra_clk_apply_init_table = tegra114_clock_apply_init_table;
> diff --git a/drivers/clk/tegra/clk-tegra124.c b/drivers/clk/tegra/clk-tegra124.c
> index 50088e976611..0c69c7970950 100644
> --- a/drivers/clk/tegra/clk-tegra124.c
> +++ b/drivers/clk/tegra/clk-tegra124.c
> @@ -1479,7 +1479,7 @@ static void __init tegra124_132_clock_init_post(struct device_node *np)
>  				  &pll_x_params);
>  	tegra_init_special_resets(1, tegra124_reset_assert,
>  				  tegra124_reset_deassert);
> -	tegra_add_of_provider(np);
> +	tegra_add_of_provider(np, of_clk_src_onecell_get);
>  
>  	clks[TEGRA124_CLK_EMC] = tegra_clk_register_emc(clk_base, np,
>  							&emc_lock);
> diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-tegra20.c
> index 636500a98561..cc857d4d4a86 100644
> --- a/drivers/clk/tegra/clk-tegra20.c
> +++ b/drivers/clk/tegra/clk-tegra20.c
> @@ -1089,6 +1089,36 @@ static const struct of_device_id pmc_match[] __initconst = {
>  	{ },
>  };
>  
> +static struct clk *tegra20_clk_src_onecell_get(struct of_phandle_args *clkspec,
> +					       void *data)
> +{
> +	struct clk_hw *parent_hw;
> +	struct clk_hw *hw;
> +	struct clk *clk;
> +
> +	clk = of_clk_src_onecell_get(clkspec, data);
> +	if (IS_ERR(clk))
> +		return clk;
> +
> +	/*
> +	 * Tegra20 CDEV1 and CDEV2 clocks are a bit special case, their parent
> +	 * clock is created by the pinctrl driver. It is possible for clk user
> +	 * to request these clocks before pinctrl driver got probed and hence
> +	 * user will get an orphaned clock. That might be undesirable because
> +	 * user may expect parent clock to be enabled by the child.
> +	 */
> +	if (clkspec->args[0] == TEGRA20_CLK_CDEV1 ||
> +	    clkspec->args[0] == TEGRA20_CLK_CDEV2) {
> +		hw = __clk_get_hw(clk);
> +
> +		parent_hw = clk_hw_get_parent(hw);
> +		if (!parent_hw)
> +			return ERR_PTR(-EPROBE_DEFER);
> +	}
> +
> +	return clk;
> +}
> +
>  static void __init tegra20_clock_init(struct device_node *np)
>  {
>  	struct device_node *node;
> @@ -1127,7 +1157,7 @@ static void __init tegra20_clock_init(struct device_node *np)
>  
>  	tegra_init_dup_clks(tegra_clk_duplicates, clks, TEGRA20_CLK_CLK_MAX);
>  
> -	tegra_add_of_provider(np);
> +	tegra_add_of_provider(np, tegra20_clk_src_onecell_get);
>  	tegra_register_devclks(devclks, ARRAY_SIZE(devclks));
>  
>  	tegra_clk_apply_init_table = tegra20_clock_apply_init_table;
> diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c
> index 9fb5d51ccce4..5435d01c636a 100644
> --- a/drivers/clk/tegra/clk-tegra210.c
> +++ b/drivers/clk/tegra/clk-tegra210.c
> @@ -3567,7 +3567,7 @@ static void __init tegra210_clock_init(struct device_node *np)
>  	tegra_init_special_resets(2, tegra210_reset_assert,
>  				  tegra210_reset_deassert);
>  
> -	tegra_add_of_provider(np);
> +	tegra_add_of_provider(np, of_clk_src_onecell_get);
>  	tegra_register_devclks(devclks, ARRAY_SIZE(devclks));
>  
>  	tegra210_mbist_clk_init();
> diff --git a/drivers/clk/tegra/clk-tegra30.c b/drivers/clk/tegra/clk-tegra30.c
> index b316dfb6f6c7..acfe661b2ae7 100644
> --- a/drivers/clk/tegra/clk-tegra30.c
> +++ b/drivers/clk/tegra/clk-tegra30.c
> @@ -1349,7 +1349,7 @@ static void __init tegra30_clock_init(struct device_node *np)
>  
>  	tegra_init_dup_clks(tegra_clk_duplicates, clks, TEGRA30_CLK_CLK_MAX);
>  
> -	tegra_add_of_provider(np);
> +	tegra_add_of_provider(np, of_clk_src_onecell_get);
>  	tegra_register_devclks(devclks, ARRAY_SIZE(devclks));
>  
>  	tegra_clk_apply_init_table = tegra30_clock_apply_init_table;
> diff --git a/drivers/clk/tegra/clk.c b/drivers/clk/tegra/clk.c
> index ba923f0d5953..593d76a114f9 100644
> --- a/drivers/clk/tegra/clk.c
> +++ b/drivers/clk/tegra/clk.c
> @@ -298,7 +298,8 @@ static struct reset_controller_dev rst_ctlr = {
>  	.of_reset_n_cells = 1,
>  };
>  
> -void __init tegra_add_of_provider(struct device_node *np)
> +void __init tegra_add_of_provider(struct device_node *np,
> +				  void *clk_src_onecell_get)
>  {
>  	int i;
>  
> @@ -314,7 +315,7 @@ void __init tegra_add_of_provider(struct device_node *np)
>  
>  	clk_data.clks = clks;
>  	clk_data.clk_num = clk_num;
> -	of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);
> +	of_clk_add_provider(np, clk_src_onecell_get, &clk_data);
>  
>  	rst_ctlr.of_node = np;
>  	rst_ctlr.nr_resets = periph_banks * 32 + num_special_reset;
> diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
> index ba7e20e6a82b..e1f88463b600 100644
> --- a/drivers/clk/tegra/clk.h
> +++ b/drivers/clk/tegra/clk.h
> @@ -763,7 +763,7 @@ struct clk **tegra_clk_init(void __iomem *clk_base, int num, int periph_banks);
>  
>  struct clk **tegra_lookup_dt_id(int clk_id, struct tegra_clk *tegra_clk);
>  
> -void tegra_add_of_provider(struct device_node *np);
> +void tegra_add_of_provider(struct device_node *np, void *clk_src_onecell_get);
>  void tegra_register_devclks(struct tegra_devclk *dev_clks, int num);
>  
>  void tegra_audio_clk_init(void __iomem *clk_base,
> -- 
> 2.17.0
> 

Re: [PATCH v3 3/4] clk: tegra: Add quirk for getting CDEV1/2 clocks on Tegra20

[Peter De Schrijver]

On Tue, May 08, 2018 at 07:26:06PM +0300, Dmitry Osipenko wrote:
> CDEV1 and CDEV2 clocks are a bit special case, their parent clock is
> created by the pinctrl driver. It should be possible for clk user to
> request these clocks before pinctrl driver got probed and hence user will
> get an orphaned clock. That might be undesirable because user may expect
> parent clock to be enabled by the child, so let's return -EPROBE_DEFER
> till parent clock appears.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>

Acked-by: Peter De Schrijver <pdeschrijver@nvidia.com>                                                                                                     


> ---
>  drivers/clk/tegra/clk-tegra114.c |  2 +-
>  drivers/clk/tegra/clk-tegra124.c |  2 +-
>  drivers/clk/tegra/clk-tegra20.c  | 32 +++++++++++++++++++++++++++++++-
>  drivers/clk/tegra/clk-tegra210.c |  2 +-
>  drivers/clk/tegra/clk-tegra30.c  |  2 +-
>  drivers/clk/tegra/clk.c          |  5 +++--
>  drivers/clk/tegra/clk.h          |  2 +-
>  7 files changed, 39 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/clk/tegra/clk-tegra114.c b/drivers/clk/tegra/clk-tegra114.c
> index 5d5a22d529f5..1824f014202b 100644
> --- a/drivers/clk/tegra/clk-tegra114.c
> +++ b/drivers/clk/tegra/clk-tegra114.c
> @@ -1367,7 +1367,7 @@ static void __init tegra114_clock_init(struct device_node *np)
>  	tegra_super_clk_gen4_init(clk_base, pmc_base, tegra114_clks,
>  					&pll_x_params);
>  
> -	tegra_add_of_provider(np);
> +	tegra_add_of_provider(np, of_clk_src_onecell_get);
>  	tegra_register_devclks(devclks, ARRAY_SIZE(devclks));
>  
>  	tegra_clk_apply_init_table = tegra114_clock_apply_init_table;
> diff --git a/drivers/clk/tegra/clk-tegra124.c b/drivers/clk/tegra/clk-tegra124.c
> index 50088e976611..0c69c7970950 100644
> --- a/drivers/clk/tegra/clk-tegra124.c
> +++ b/drivers/clk/tegra/clk-tegra124.c
> @@ -1479,7 +1479,7 @@ static void __init tegra124_132_clock_init_post(struct device_node *np)
>  				  &pll_x_params);
>  	tegra_init_special_resets(1, tegra124_reset_assert,
>  				  tegra124_reset_deassert);
> -	tegra_add_of_provider(np);
> +	tegra_add_of_provider(np, of_clk_src_onecell_get);
>  
>  	clks[TEGRA124_CLK_EMC] = tegra_clk_register_emc(clk_base, np,
>  							&emc_lock);
> diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-tegra20.c
> index 636500a98561..cc857d4d4a86 100644
> --- a/drivers/clk/tegra/clk-tegra20.c
> +++ b/drivers/clk/tegra/clk-tegra20.c
> @@ -1089,6 +1089,36 @@ static const struct of_device_id pmc_match[] __initconst = {
>  	{ },
>  };
>  
> +static struct clk *tegra20_clk_src_onecell_get(struct of_phandle_args *clkspec,
> +					       void *data)
> +{
> +	struct clk_hw *parent_hw;
> +	struct clk_hw *hw;
> +	struct clk *clk;
> +
> +	clk = of_clk_src_onecell_get(clkspec, data);
> +	if (IS_ERR(clk))
> +		return clk;
> +
> +	/*
> +	 * Tegra20 CDEV1 and CDEV2 clocks are a bit special case, their parent
> +	 * clock is created by the pinctrl driver. It is possible for clk user
> +	 * to request these clocks before pinctrl driver got probed and hence
> +	 * user will get an orphaned clock. That might be undesirable because
> +	 * user may expect parent clock to be enabled by the child.
> +	 */
> +	if (clkspec->args[0] == TEGRA20_CLK_CDEV1 ||
> +	    clkspec->args[0] == TEGRA20_CLK_CDEV2) {
> +		hw = __clk_get_hw(clk);
> +
> +		parent_hw = clk_hw_get_parent(hw);
> +		if (!parent_hw)
> +			return ERR_PTR(-EPROBE_DEFER);
> +	}
> +
> +	return clk;
> +}
> +
>  static void __init tegra20_clock_init(struct device_node *np)
>  {
>  	struct device_node *node;
> @@ -1127,7 +1157,7 @@ static void __init tegra20_clock_init(struct device_node *np)
>  
>  	tegra_init_dup_clks(tegra_clk_duplicates, clks, TEGRA20_CLK_CLK_MAX);
>  
> -	tegra_add_of_provider(np);
> +	tegra_add_of_provider(np, tegra20_clk_src_onecell_get);
>  	tegra_register_devclks(devclks, ARRAY_SIZE(devclks));
>  
>  	tegra_clk_apply_init_table = tegra20_clock_apply_init_table;
> diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c
> index 9fb5d51ccce4..5435d01c636a 100644
> --- a/drivers/clk/tegra/clk-tegra210.c
> +++ b/drivers/clk/tegra/clk-tegra210.c
> @@ -3567,7 +3567,7 @@ static void __init tegra210_clock_init(struct device_node *np)
>  	tegra_init_special_resets(2, tegra210_reset_assert,
>  				  tegra210_reset_deassert);
>  
> -	tegra_add_of_provider(np);
> +	tegra_add_of_provider(np, of_clk_src_onecell_get);
>  	tegra_register_devclks(devclks, ARRAY_SIZE(devclks));
>  
>  	tegra210_mbist_clk_init();
> diff --git a/drivers/clk/tegra/clk-tegra30.c b/drivers/clk/tegra/clk-tegra30.c
> index b316dfb6f6c7..acfe661b2ae7 100644
> --- a/drivers/clk/tegra/clk-tegra30.c
> +++ b/drivers/clk/tegra/clk-tegra30.c
> @@ -1349,7 +1349,7 @@ static void __init tegra30_clock_init(struct device_node *np)
>  
>  	tegra_init_dup_clks(tegra_clk_duplicates, clks, TEGRA30_CLK_CLK_MAX);
>  
> -	tegra_add_of_provider(np);
> +	tegra_add_of_provider(np, of_clk_src_onecell_get);
>  	tegra_register_devclks(devclks, ARRAY_SIZE(devclks));
>  
>  	tegra_clk_apply_init_table = tegra30_clock_apply_init_table;
> diff --git a/drivers/clk/tegra/clk.c b/drivers/clk/tegra/clk.c
> index ba923f0d5953..593d76a114f9 100644
> --- a/drivers/clk/tegra/clk.c
> +++ b/drivers/clk/tegra/clk.c
> @@ -298,7 +298,8 @@ static struct reset_controller_dev rst_ctlr = {
>  	.of_reset_n_cells = 1,
>  };
>  
> -void __init tegra_add_of_provider(struct device_node *np)
> +void __init tegra_add_of_provider(struct device_node *np,
> +				  void *clk_src_onecell_get)
>  {
>  	int i;
>  
> @@ -314,7 +315,7 @@ void __init tegra_add_of_provider(struct device_node *np)
>  
>  	clk_data.clks = clks;
>  	clk_data.clk_num = clk_num;
> -	of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);
> +	of_clk_add_provider(np, clk_src_onecell_get, &clk_data);
>  
>  	rst_ctlr.of_node = np;
>  	rst_ctlr.nr_resets = periph_banks * 32 + num_special_reset;
> diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
> index ba7e20e6a82b..e1f88463b600 100644
> --- a/drivers/clk/tegra/clk.h
> +++ b/drivers/clk/tegra/clk.h
> @@ -763,7 +763,7 @@ struct clk **tegra_clk_init(void __iomem *clk_base, int num, int periph_banks);
>  
>  struct clk **tegra_lookup_dt_id(int clk_id, struct tegra_clk *tegra_clk);
>  
> -void tegra_add_of_provider(struct device_node *np);
> +void tegra_add_of_provider(struct device_node *np, void *clk_src_onecell_get);
>  void tegra_register_devclks(struct tegra_devclk *dev_clks, int num);
>  
>  void tegra_audio_clk_init(void __iomem *clk_base,
> -- 
> 2.17.0
> 

Re: [PATCH v3 0/4] Restore ULPI USB on Tegra20

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2256 bytes --]

On Tue, May 08, 2018 at 07:26:02PM +0300, Dmitry Osipenko wrote:
> Hello,
> 
> This series of patches fixes ULPI USB on Tegra20. The original problem
> was reported by Marcel Ziswiler, he found that "ulpi-link" clock was
> incorrectly set to CDEV2 instead of PLL_P_OUT4. Marcel made a patch
> that changed the "ulpi-link" clock to PLL_P_OUT4 and that fixed issue
> with the USB for the devices that have CDEV2 being enabled by bootloader.
> The patch got into the kernel and later Marc Dietrich found that USB
> stopped working on the "paz00" Tegra20 board. After a bit of discussion
> was revealed that PLL_P_OUT4 is the parent clock of the CDEV2 and clock
> driver was setting CDEV2's parent incorrectly. The parent clock is actually
> determined by the pinmuxing config of CDEV2 pingroup. This patchset fixes
> the parent of CDEV2 clock by making Tegra's pinctrl driver a clock provider,
> providing CDEV1/2 clock muxes (thanks to Peter De Schrijver for the
> suggestion), and then setting these clock muxes as parents for the CDEV1/2
> clocks. In the end Marcel's CDEV2->PLL_P_OUT4 change is reverted since CDEV2
> (aka MCLK2) is the actual clock source for "ulpi-link".
> 
> Changelog:
> 
> v3:
> 	- Use clk DT ID's instead of comparing clk names and make
> 	  custom of_src_onecell_get specific to Tegra20 clk provider
> 	  in the "Add quirk for getting CDEV1/2 clocks on Tegra20" patch
> 	  as was suggested by Peter De Schrijver for v2.
> 
> v2:
> 	- Added new patch "Add quirk for getting CDEV1/2 clocks", assuring
> 	  that clk user won't get CDEV1/2 clocks until parent clk muxes
> 	  are available, i.e. resolves potential issue with CDEV-user driver
> 	  vs pinctrl driver probe order.
> 
> 	- Factored out "pinctrl" patch from the patchset as was requested by
> 	  Linus Walleij.
> 
> 	- Addressed v1 review comments: fixed swapped DEV1/2 clk div bits,
> 	  made DEV1/2 divs read-only, etc minor changes.
> 
> Dmitry Osipenko (4):
>   clk: tegra20: Add DEV1/DEV2 OSC dividers
>   clk: tegra20: Correct parents of CDEV1/2 clocks
>   clk: tegra: Add quirk for getting CDEV1/2 clocks on Tegra20
>   ARM: dts: tegra20: Revert "Fix ULPI regression on Tegra20"

Patches 1-3 applied, thanks.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3] pinctrl: tegra20: Provide CDEV1/2 clock muxes

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1233 bytes --]

On Tue, May 08, 2018 at 07:26:04PM +0300, Dmitry Osipenko wrote:
> Muxing of pins MCLK1/2 determine the muxing of the corresponding clocks.
> Make pinctrl driver to provide clock muxes for the CDEV1/2 pingroups, so
> that main clk-controller driver could get an actual parent clock for the
> CDEV1/2 clocks.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> Reviewed-by: Marcel Ziswiler <marcel@ziswiler.com>
> Tested-by: Marcel Ziswiler <marcel@ziswiler.com>
> Tested-by: Marc Dietrich <marvin24@gmx.de>
> Acked-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> ---
> 
> Changelog:
> 
> v3:
> 	- No change.
> 
> v2:
> 	- See changelog of "Restore ULPI USB on Tegra20" v2 series.
> 
>  drivers/pinctrl/tegra/pinctrl-tegra.c   | 11 ---------
>  drivers/pinctrl/tegra/pinctrl-tegra.h   | 11 +++++++++
>  drivers/pinctrl/tegra/pinctrl-tegra20.c | 30 ++++++++++++++++++++++++-
>  3 files changed, 40 insertions(+), 12 deletions(-)

Linus,

were you going to pick this up for v4.18? There's a runtime dependency
on this from some clock patches that I'd like to get into v4.18.

I see I never gave my Acked-by on this particular patch, so here goes:

Acked-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3] pinctrl: tegra20: Provide CDEV1/2 clock muxes

[Dmitry Osipenko]

On 18.05.2018 13:39, Thierry Reding wrote:
> On Tue, May 08, 2018 at 07:26:04PM +0300, Dmitry Osipenko wrote:
>> Muxing of pins MCLK1/2 determine the muxing of the corresponding clocks.
>> Make pinctrl driver to provide clock muxes for the CDEV1/2 pingroups, so
>> that main clk-controller driver could get an actual parent clock for the
>> CDEV1/2 clocks.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> Reviewed-by: Marcel Ziswiler <marcel@ziswiler.com>
>> Tested-by: Marcel Ziswiler <marcel@ziswiler.com>
>> Tested-by: Marc Dietrich <marvin24@gmx.de>
>> Acked-by: Peter De Schrijver <pdeschrijver@nvidia.com>
>> ---
>>
>> Changelog:
>>
>> v3:
>> 	- No change.
>>
>> v2:
>> 	- See changelog of "Restore ULPI USB on Tegra20" v2 series.
>>
>>  drivers/pinctrl/tegra/pinctrl-tegra.c   | 11 ---------
>>  drivers/pinctrl/tegra/pinctrl-tegra.h   | 11 +++++++++
>>  drivers/pinctrl/tegra/pinctrl-tegra20.c | 30 ++++++++++++++++++++++++-
>>  3 files changed, 40 insertions(+), 12 deletions(-)
> 
> Linus,
> 
> were you going to pick this up for v4.18? There's a runtime dependency
> on this from some clock patches that I'd like to get into v4.18.
> 
> I see I never gave my Acked-by on this particular patch, so here goes:
> 
> Acked-by: Thierry Reding <treding@nvidia.com>

Linus applied the v2 of the patch [0]. Thanks for caring about it.

[0] https://www.spinics.net/lists/linux-tegra/msg33331.html