2019-11-27 05:03:20

by Sowjanya Komatineni

[permalink] [raw]
Subject: [PATCH v2 00/11] Move PMC clocks into Tegra PMC driver

Tegra PMC has clk_out_1, clk_out_2, clk_out_3 and blink controls which
are currently registered by Tegra clock driver using clk_regiser_mux and
clk_register_gate which performs direct Tegra PMC register access.

When Tegra PMC is in secure mode, any access from non-secure world will
not go through.

This patch series adds these Tegra PMC clocks and blink controls to Tegra
PMC driver with PMC as clock provider and removed them from Tegra clock
driver. This also adds PMC specific clock id's to use in device tree and
removed clock ids of PMC clock from Tegra clock driver.

This series also includes patch to update clock provider from tegra_car
to pmc in the device tree tegra210-smaug.dts that uses clk_out_2 from PMC.

[v2]: Changes between v1 and v2 are
- v2 includes patches for adding clk_out_1, clk_out_2, clk_out_3,
blink controls to Tegra PMC driver and removing clk-tegra-pmc.
- feedback related to pmc clocks in Tegra PMC driver from v1
- Removed patches for WB0 PLLM overrides and PLLE IDDQ PMC programming
by the clock driver using helper functions from Tegra PMC.

Note:
To use helper functions from PMC driver, PMC early init need to
happen prior to using helper functions and these helper functions are
for PLLM Override and PLLE IDDQ programming in PMC during PLLM/PLLE
clock registration which happen in clock_init prior to Tegra PMC
probe.
Moving PLLM/PLLE clocks registration to happen after Tegra PMC
impacts other clocks EMC, MC and corresponding tegra_emc_init and
tegra_mc_init.
This implementation of configuring PMC registers thru helper
functions in clock driver needs proper changes across PMC, Clock,
EMC and MC inits to have it work across all Tegra platforms.

Currently PLLM Override is not enabled in the bootloader so proper
patches for this fix will be taken care separately.

[v1]: v1 includes patches for below fixes.
- adding clk_out_1, clk_out_2, clk_out_3, blink controls to Tegra PMC
driver and removing clk-tegra-pmc.
- updated clock provider from tegra_car to pmc in the device tree
tegra210-smaug.dts that uses clk_out_2.
- Added helper functions in PMC driver for WB0 PLLM overrides and PLLE
IDDQ programming to use by clock driver and updated clock driver to
use these helper functions and removed direct PMC access from clock
driver and all pmc base address references in clock driver.


Sowjanya Komatineni (11):
dt-bindings: soc: tegra-pmc: Add Tegra PMC clock ids
soc: tegra: Add Tegra PMC clock registrations into PMC driver
dt-bindings: soc: tegra-pmc: Add id for Tegra PMC blink control
soc: pmc: Add blink output clock registration to Tegra PMC
clk: tegra: Remove tegra_pmc_clk_init along with clk ids
dt-bindings: clock: tegra: Remove pmc clock ids from clock dt-bindings
arm: tegra: Add clock-cells property to Tegra PMC
arm64: tegra: Add clock-cells property to Tegra pmc
dt-bindings: Add Tegra PMC clock configuration bindings
arm64: tegra: smaug: Change clk_out_2 provider from tegra_car to pmc
ASoC: nau8825: change Tegra clk_out_2 provider from tegra_car to pmc

.../bindings/arm/tegra/nvidia,tegra20-pmc.txt | 45 ++-
.../devicetree/bindings/sound/nau8825.txt | 2 +-
arch/arm/boot/dts/tegra114.dtsi | 4 +-
arch/arm/boot/dts/tegra124.dtsi | 4 +-
arch/arm/boot/dts/tegra20.dtsi | 4 +-
arch/arm/boot/dts/tegra30.dtsi | 4 +-
arch/arm64/boot/dts/nvidia/tegra132.dtsi | 4 +-
arch/arm64/boot/dts/nvidia/tegra186.dtsi | 2 +
arch/arm64/boot/dts/nvidia/tegra194.dtsi | 2 +
arch/arm64/boot/dts/nvidia/tegra210-smaug.dts | 2 +-
arch/arm64/boot/dts/nvidia/tegra210.dtsi | 2 +
drivers/clk/tegra/Makefile | 1 -
drivers/clk/tegra/clk-id.h | 7 -
drivers/clk/tegra/clk-tegra-pmc.c | 122 -------
drivers/clk/tegra/clk-tegra114.c | 11 -
drivers/clk/tegra/clk-tegra124.c | 27 +-
drivers/clk/tegra/clk-tegra20.c | 4 -
drivers/clk/tegra/clk-tegra210.c | 11 -
drivers/clk/tegra/clk-tegra30.c | 12 -
drivers/clk/tegra/clk.h | 1 -
drivers/soc/tegra/pmc.c | 379 +++++++++++++++++++++
include/dt-bindings/clock/tegra114-car.h | 14 +-
include/dt-bindings/clock/tegra124-car-common.h | 14 +-
include/dt-bindings/clock/tegra20-car.h | 2 +-
include/dt-bindings/clock/tegra210-car.h | 14 +-
include/dt-bindings/clock/tegra30-car.h | 14 +-
include/dt-bindings/soc/tegra-pmc.h | 19 ++
27 files changed, 501 insertions(+), 226 deletions(-)
delete mode 100644 drivers/clk/tegra/clk-tegra-pmc.c
create mode 100644 include/dt-bindings/soc/tegra-pmc.h

--
2.7.4


2019-11-27 05:04:20

by Sowjanya Komatineni

[permalink] [raw]
Subject: [PATCH v2 08/11] arm64: tegra: Add clock-cells property to Tegra pmc

Tegra pmc has 3 clocks clk_out_1, clk_out_2, clk_out_3 with mux and gate
for each of these clocks as part of pmc and Tegra pmc is the clock provider
for these clocks.

These clock ids are part of pmc dt-bindings.

This patch includes pmc dt-bindings and adds #clock-cells propert
with 1 clock specifier to Tegra pmc node.

Signed-off-by: Sowjanya Komatineni <[email protected]>
---
arch/arm64/boot/dts/nvidia/tegra132.dtsi | 4 +++-
arch/arm64/boot/dts/nvidia/tegra186.dtsi | 2 ++
arch/arm64/boot/dts/nvidia/tegra194.dtsi | 2 ++
arch/arm64/boot/dts/nvidia/tegra210.dtsi | 2 ++
4 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/nvidia/tegra132.dtsi b/arch/arm64/boot/dts/nvidia/tegra132.dtsi
index 631a7f77c386..5bdb4a6a6b90 100644
--- a/arch/arm64/boot/dts/nvidia/tegra132.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra132.dtsi
@@ -6,6 +6,7 @@
#include <dt-bindings/pinctrl/pinctrl-tegra-xusb.h>
#include <dt-bindings/interrupt-controller/arm-gic.h>
#include <dt-bindings/thermal/tegra124-soctherm.h>
+#include <dt-bindings/soc/tegra-pmc.h>

/ {
compatible = "nvidia,tegra132", "nvidia,tegra124";
@@ -577,11 +578,12 @@
clock-names = "rtc";
};

- pmc@7000e400 {
+ pmc: pmc@7000e400 {
compatible = "nvidia,tegra124-pmc";
reg = <0x0 0x7000e400 0x0 0x400>;
clocks = <&tegra_car TEGRA124_CLK_PCLK>, <&clk32k_in>;
clock-names = "pclk", "clk32k_in";
+ #clock-cells = <1>;
};

fuse@7000f800 {
diff --git a/arch/arm64/boot/dts/nvidia/tegra186.dtsi b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
index 7893d78a0fb6..627108ce2f56 100644
--- a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
@@ -8,6 +8,7 @@
#include <dt-bindings/power/tegra186-powergate.h>
#include <dt-bindings/reset/tegra186-reset.h>
#include <dt-bindings/thermal/tegra186-bpmp-thermal.h>
+#include <dt-bindings/soc/tegra-pmc.h>

/ {
compatible = "nvidia,tegra186";
@@ -670,6 +671,7 @@
<0 0x0c390000 0 0x10000>;
reg-names = "pmc", "wake", "aotag", "scratch";

+ #clock-cells = <1>;
#interrupt-cells = <2>;
interrupt-controller;

diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
index 11220d97adb8..37dc19f49e4f 100644
--- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
@@ -7,6 +7,7 @@
#include <dt-bindings/power/tegra194-powergate.h>
#include <dt-bindings/reset/tegra194-reset.h>
#include <dt-bindings/thermal/tegra194-bpmp-thermal.h>
+#include <dt-bindings/soc/tegra-pmc.h>

/ {
compatible = "nvidia,tegra194";
@@ -799,6 +800,7 @@
<0x0c3a0000 0x10000>;
reg-names = "pmc", "wake", "aotag", "scratch", "misc";

+ #clock-cells = <1>;
#interrupt-cells = <2>;
interrupt-controller;
};
diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
index 48c63256ba7f..0d0432d3b37a 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
@@ -7,6 +7,7 @@
#include <dt-bindings/reset/tegra210-car.h>
#include <dt-bindings/interrupt-controller/arm-gic.h>
#include <dt-bindings/thermal/tegra124-soctherm.h>
+#include <dt-bindings/soc/tegra-pmc.h>

/ {
compatible = "nvidia,tegra210";
@@ -780,6 +781,7 @@
reg = <0x0 0x7000e400 0x0 0x400>;
clocks = <&tegra_car TEGRA210_CLK_PCLK>, <&clk32k_in>;
clock-names = "pclk", "clk32k_in";
+ #clock-cells = <1>;
#interrupt-cells = <2>;
interrupt-controller;

--
2.7.4

2019-11-27 05:05:00

by Sowjanya Komatineni

[permalink] [raw]
Subject: [PATCH v2 06/11] dt-bindings: clock: tegra: Remove pmc clock ids from clock dt-bindings

clk_out_1, clk_out_2, clk_out_3, blink are part of Tegra pmc clocks.

This patch removes ids for these clocks from Tegra clock dt-bindings.

Signed-off-by: Sowjanya Komatineni <[email protected]>
---
include/dt-bindings/clock/tegra114-car.h | 14 +++++++-------
include/dt-bindings/clock/tegra124-car-common.h | 14 +++++++-------
include/dt-bindings/clock/tegra20-car.h | 2 +-
include/dt-bindings/clock/tegra210-car.h | 14 +++++++-------
include/dt-bindings/clock/tegra30-car.h | 14 +++++++-------
5 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/include/dt-bindings/clock/tegra114-car.h b/include/dt-bindings/clock/tegra114-car.h
index bb5c2c999c05..9175cd0571b5 100644
--- a/include/dt-bindings/clock/tegra114-car.h
+++ b/include/dt-bindings/clock/tegra114-car.h
@@ -270,10 +270,10 @@
#define TEGRA114_CLK_AUDIO3 242
#define TEGRA114_CLK_AUDIO4 243
#define TEGRA114_CLK_SPDIF 244
-#define TEGRA114_CLK_CLK_OUT_1 245
-#define TEGRA114_CLK_CLK_OUT_2 246
-#define TEGRA114_CLK_CLK_OUT_3 247
-#define TEGRA114_CLK_BLINK 248
+/* 245 */
+/* 246 */
+/* 247 */
+/* 248 */
/* 249 */
/* 250 */
/* 251 */
@@ -333,9 +333,9 @@
#define TEGRA114_CLK_AUDIO3_MUX 303
#define TEGRA114_CLK_AUDIO4_MUX 304
#define TEGRA114_CLK_SPDIF_MUX 305
-#define TEGRA114_CLK_CLK_OUT_1_MUX 306
-#define TEGRA114_CLK_CLK_OUT_2_MUX 307
-#define TEGRA114_CLK_CLK_OUT_3_MUX 308
+/* 306 */
+/* 307 */
+/* 308 */
#define TEGRA114_CLK_DSIA_MUX 309
#define TEGRA114_CLK_DSIB_MUX 310
#define TEGRA114_CLK_XUSB_SS_DIV2 311
diff --git a/include/dt-bindings/clock/tegra124-car-common.h b/include/dt-bindings/clock/tegra124-car-common.h
index 0c4f5be0a742..90a0c5e7eb5f 100644
--- a/include/dt-bindings/clock/tegra124-car-common.h
+++ b/include/dt-bindings/clock/tegra124-car-common.h
@@ -269,10 +269,10 @@
#define TEGRA124_CLK_AUDIO3 242
#define TEGRA124_CLK_AUDIO4 243
#define TEGRA124_CLK_SPDIF 244
-#define TEGRA124_CLK_CLK_OUT_1 245
-#define TEGRA124_CLK_CLK_OUT_2 246
-#define TEGRA124_CLK_CLK_OUT_3 247
-#define TEGRA124_CLK_BLINK 248
+/* 245 */
+/* 246 */
+/* 247 */
+/* 248 */
/* 249 */
/* 250 */
/* 251 */
@@ -332,9 +332,9 @@
#define TEGRA124_CLK_AUDIO3_MUX 303
#define TEGRA124_CLK_AUDIO4_MUX 304
#define TEGRA124_CLK_SPDIF_MUX 305
-#define TEGRA124_CLK_CLK_OUT_1_MUX 306
-#define TEGRA124_CLK_CLK_OUT_2_MUX 307
-#define TEGRA124_CLK_CLK_OUT_3_MUX 308
+/* 306 */
+/* 307 */
+/* 308 */
/* 309 */
/* 310 */
#define TEGRA124_CLK_SOR0_LVDS 311 /* deprecated */
diff --git a/include/dt-bindings/clock/tegra20-car.h b/include/dt-bindings/clock/tegra20-car.h
index b21a0eb32921..fe541f627965 100644
--- a/include/dt-bindings/clock/tegra20-car.h
+++ b/include/dt-bindings/clock/tegra20-car.h
@@ -131,7 +131,7 @@
#define TEGRA20_CLK_CCLK 108
#define TEGRA20_CLK_HCLK 109
#define TEGRA20_CLK_PCLK 110
-#define TEGRA20_CLK_BLINK 111
+/* 111 */
#define TEGRA20_CLK_PLL_A 112
#define TEGRA20_CLK_PLL_A_OUT0 113
#define TEGRA20_CLK_PLL_C 114
diff --git a/include/dt-bindings/clock/tegra210-car.h b/include/dt-bindings/clock/tegra210-car.h
index 44f60623f99b..a3d8d3e75728 100644
--- a/include/dt-bindings/clock/tegra210-car.h
+++ b/include/dt-bindings/clock/tegra210-car.h
@@ -304,10 +304,10 @@
#define TEGRA210_CLK_AUDIO3 274
#define TEGRA210_CLK_AUDIO4 275
#define TEGRA210_CLK_SPDIF 276
-#define TEGRA210_CLK_CLK_OUT_1 277
-#define TEGRA210_CLK_CLK_OUT_2 278
-#define TEGRA210_CLK_CLK_OUT_3 279
-#define TEGRA210_CLK_BLINK 280
+/* 277 */
+/* 278 */
+/* 279 */
+/* 280 */
#define TEGRA210_CLK_SOR0_LVDS 281 /* deprecated */
#define TEGRA210_CLK_SOR0_OUT 281
#define TEGRA210_CLK_SOR1_OUT 282
@@ -386,9 +386,9 @@
#define TEGRA210_CLK_AUDIO3_MUX 353
#define TEGRA210_CLK_AUDIO4_MUX 354
#define TEGRA210_CLK_SPDIF_MUX 355
-#define TEGRA210_CLK_CLK_OUT_1_MUX 356
-#define TEGRA210_CLK_CLK_OUT_2_MUX 357
-#define TEGRA210_CLK_CLK_OUT_3_MUX 358
+/* 356 */
+/* 357 */
+/* 358 */
#define TEGRA210_CLK_DSIA_MUX 359
#define TEGRA210_CLK_DSIB_MUX 360
/* 361 */
diff --git a/include/dt-bindings/clock/tegra30-car.h b/include/dt-bindings/clock/tegra30-car.h
index 3c90f1535551..20ef2462d9e1 100644
--- a/include/dt-bindings/clock/tegra30-car.h
+++ b/include/dt-bindings/clock/tegra30-car.h
@@ -230,11 +230,11 @@
#define TEGRA30_CLK_AUDIO3 204
#define TEGRA30_CLK_AUDIO4 205
#define TEGRA30_CLK_SPDIF 206
-#define TEGRA30_CLK_CLK_OUT_1 207 /* (extern1) */
-#define TEGRA30_CLK_CLK_OUT_2 208 /* (extern2) */
-#define TEGRA30_CLK_CLK_OUT_3 209 /* (extern3) */
+/* 207 */
+/* 208 */
+/* 209 */
#define TEGRA30_CLK_SCLK 210
-#define TEGRA30_CLK_BLINK 211
+/* 211 */
#define TEGRA30_CLK_CCLK_G 212
#define TEGRA30_CLK_CCLK_LP 213
#define TEGRA30_CLK_TWD 214
@@ -260,9 +260,9 @@
/* 297 */
/* 298 */
/* 299 */
-#define TEGRA30_CLK_CLK_OUT_1_MUX 300
-#define TEGRA30_CLK_CLK_OUT_2_MUX 301
-#define TEGRA30_CLK_CLK_OUT_3_MUX 302
+/* 300 */
+/* 301 */
+/* 302 */
#define TEGRA30_CLK_AUDIO0_MUX 303
#define TEGRA30_CLK_AUDIO1_MUX 304
#define TEGRA30_CLK_AUDIO2_MUX 305
--
2.7.4

2019-11-27 14:33:39

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH v2 00/11] Move PMC clocks into Tegra PMC driver

27.11.2019 07:59, Sowjanya Komatineni пишет:
> Tegra PMC has clk_out_1, clk_out_2, clk_out_3 and blink controls which
> are currently registered by Tegra clock driver using clk_regiser_mux and
> clk_register_gate which performs direct Tegra PMC register access.
>
> When Tegra PMC is in secure mode, any access from non-secure world will
> not go through.
>
> This patch series adds these Tegra PMC clocks and blink controls to Tegra
> PMC driver with PMC as clock provider and removed them from Tegra clock
> driver. This also adds PMC specific clock id's to use in device tree and
> removed clock ids of PMC clock from Tegra clock driver.
>
> This series also includes patch to update clock provider from tegra_car
> to pmc in the device tree tegra210-smaug.dts that uses clk_out_2 from PMC.
>
> [v2]: Changes between v1 and v2 are
> - v2 includes patches for adding clk_out_1, clk_out_2, clk_out_3,
> blink controls to Tegra PMC driver and removing clk-tegra-pmc.
> - feedback related to pmc clocks in Tegra PMC driver from v1
> - Removed patches for WB0 PLLM overrides and PLLE IDDQ PMC programming
> by the clock driver using helper functions from Tegra PMC.
>
> Note:
> To use helper functions from PMC driver, PMC early init need to
> happen prior to using helper functions and these helper functions are
> for PLLM Override and PLLE IDDQ programming in PMC during PLLM/PLLE
> clock registration which happen in clock_init prior to Tegra PMC
> probe.
> Moving PLLM/PLLE clocks registration to happen after Tegra PMC
> impacts other clocks EMC, MC and corresponding tegra_emc_init and
> tegra_mc_init.
> This implementation of configuring PMC registers thru helper
> functions in clock driver needs proper changes across PMC, Clock,
> EMC and MC inits to have it work across all Tegra platforms.
>
> Currently PLLM Override is not enabled in the bootloader so proper
> patches for this fix will be taken care separately.

Hello Sowjanya,

Could you please clarify what do you mean by "PLLM Override not enabled
in bootloader"?

There is T124 Nyan Big Chromebook which is supported in upstream kernel,
it has PLLM Override set by bootloader. I also have T30 Nexus 7 tablet
which has the PLLM Override set by bootloader as well. It's not clear to
me whether this patch series is supposed to break these devices. If the
breakage is the case here, then I'm afraid you can't postpone supporting
the PLLM Override and a full-featured implementation is needed.

I briefly tried to test this series on T30 and this time it doesn't hang
on boot, but somehow WiFi MMC card detection is broken. AFAIK, the WiFi
chip uses the Blink clock source and the clock should be enabled by the
MMC core because this is how DT part looks like:

brcm_wifi_pwrseq: wifi-pwrseq {
compatible = "mmc-pwrseq-simple";
clocks = <&pmc TEGRA_PMC_CLK_BLINK>;
clock-names = "ext_clock";
reset-gpios = <&gpio TEGRA_GPIO(D, 3) GPIO_ACTIVE_LOW>;
post-power-on-delay-ms = <300>;
power-off-delay-us = <300>;
};

BTW, I tried this series on a T20 device which also uses the Blink
clock for WiFi card and it works. So looks like this patchset has some
problem in regards to the T30 PMC clocks implementation.

[snip]

2019-11-27 14:34:58

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH v2 06/11] dt-bindings: clock: tegra: Remove pmc clock ids from clock dt-bindings

27.11.2019 07:59, Sowjanya Komatineni пишет:
> clk_out_1, clk_out_2, clk_out_3, blink are part of Tegra pmc clocks.
>
> This patch removes ids for these clocks from Tegra clock dt-bindings.
>
> Signed-off-by: Sowjanya Komatineni <[email protected]>
> ---
> include/dt-bindings/clock/tegra114-car.h | 14 +++++++-------
> include/dt-bindings/clock/tegra124-car-common.h | 14 +++++++-------
> include/dt-bindings/clock/tegra20-car.h | 2 +-
> include/dt-bindings/clock/tegra210-car.h | 14 +++++++-------
> include/dt-bindings/clock/tegra30-car.h | 14 +++++++-------
> 5 files changed, 29 insertions(+), 29 deletions(-)
>
> diff --git a/include/dt-bindings/clock/tegra114-car.h b/include/dt-bindings/clock/tegra114-car.h
> index bb5c2c999c05..9175cd0571b5 100644
> --- a/include/dt-bindings/clock/tegra114-car.h
> +++ b/include/dt-bindings/clock/tegra114-car.h
> @@ -270,10 +270,10 @@
> #define TEGRA114_CLK_AUDIO3 242
> #define TEGRA114_CLK_AUDIO4 243
> #define TEGRA114_CLK_SPDIF 244
> -#define TEGRA114_CLK_CLK_OUT_1 245
> -#define TEGRA114_CLK_CLK_OUT_2 246
> -#define TEGRA114_CLK_CLK_OUT_3 247
> -#define TEGRA114_CLK_BLINK 248
> +/* 245 */
> +/* 246 */
> +/* 247 */
> +/* 248 */
> /* 249 */
> /* 250 */
> /* 251 */
> @@ -333,9 +333,9 @@
> #define TEGRA114_CLK_AUDIO3_MUX 303
> #define TEGRA114_CLK_AUDIO4_MUX 304
> #define TEGRA114_CLK_SPDIF_MUX 305
> -#define TEGRA114_CLK_CLK_OUT_1_MUX 306
> -#define TEGRA114_CLK_CLK_OUT_2_MUX 307
> -#define TEGRA114_CLK_CLK_OUT_3_MUX 308
> +/* 306 */
> +/* 307 */
> +/* 308 */
> #define TEGRA114_CLK_DSIA_MUX 309
> #define TEGRA114_CLK_DSIB_MUX 310
> #define TEGRA114_CLK_XUSB_SS_DIV2 311
> diff --git a/include/dt-bindings/clock/tegra124-car-common.h b/include/dt-bindings/clock/tegra124-car-common.h
> index 0c4f5be0a742..90a0c5e7eb5f 100644
> --- a/include/dt-bindings/clock/tegra124-car-common.h
> +++ b/include/dt-bindings/clock/tegra124-car-common.h
> @@ -269,10 +269,10 @@
> #define TEGRA124_CLK_AUDIO3 242
> #define TEGRA124_CLK_AUDIO4 243
> #define TEGRA124_CLK_SPDIF 244
> -#define TEGRA124_CLK_CLK_OUT_1 245
> -#define TEGRA124_CLK_CLK_OUT_2 246
> -#define TEGRA124_CLK_CLK_OUT_3 247
> -#define TEGRA124_CLK_BLINK 248
> +/* 245 */
> +/* 246 */
> +/* 247 */
> +/* 248 */
> /* 249 */
> /* 250 */
> /* 251 */
> @@ -332,9 +332,9 @@
> #define TEGRA124_CLK_AUDIO3_MUX 303
> #define TEGRA124_CLK_AUDIO4_MUX 304
> #define TEGRA124_CLK_SPDIF_MUX 305
> -#define TEGRA124_CLK_CLK_OUT_1_MUX 306
> -#define TEGRA124_CLK_CLK_OUT_2_MUX 307
> -#define TEGRA124_CLK_CLK_OUT_3_MUX 308
> +/* 306 */
> +/* 307 */
> +/* 308 */
> /* 309 */
> /* 310 */
> #define TEGRA124_CLK_SOR0_LVDS 311 /* deprecated */
> diff --git a/include/dt-bindings/clock/tegra20-car.h b/include/dt-bindings/clock/tegra20-car.h
> index b21a0eb32921..fe541f627965 100644
> --- a/include/dt-bindings/clock/tegra20-car.h
> +++ b/include/dt-bindings/clock/tegra20-car.h
> @@ -131,7 +131,7 @@
> #define TEGRA20_CLK_CCLK 108
> #define TEGRA20_CLK_HCLK 109
> #define TEGRA20_CLK_PCLK 110
> -#define TEGRA20_CLK_BLINK 111
> +/* 111 */
> #define TEGRA20_CLK_PLL_A 112
> #define TEGRA20_CLK_PLL_A_OUT0 113
> #define TEGRA20_CLK_PLL_C 114
> diff --git a/include/dt-bindings/clock/tegra210-car.h b/include/dt-bindings/clock/tegra210-car.h
> index 44f60623f99b..a3d8d3e75728 100644
> --- a/include/dt-bindings/clock/tegra210-car.h
> +++ b/include/dt-bindings/clock/tegra210-car.h
> @@ -304,10 +304,10 @@
> #define TEGRA210_CLK_AUDIO3 274
> #define TEGRA210_CLK_AUDIO4 275
> #define TEGRA210_CLK_SPDIF 276
> -#define TEGRA210_CLK_CLK_OUT_1 277
> -#define TEGRA210_CLK_CLK_OUT_2 278
> -#define TEGRA210_CLK_CLK_OUT_3 279
> -#define TEGRA210_CLK_BLINK 280
> +/* 277 */
> +/* 278 */
> +/* 279 */
> +/* 280 */
> #define TEGRA210_CLK_SOR0_LVDS 281 /* deprecated */
> #define TEGRA210_CLK_SOR0_OUT 281
> #define TEGRA210_CLK_SOR1_OUT 282
> @@ -386,9 +386,9 @@
> #define TEGRA210_CLK_AUDIO3_MUX 353
> #define TEGRA210_CLK_AUDIO4_MUX 354
> #define TEGRA210_CLK_SPDIF_MUX 355
> -#define TEGRA210_CLK_CLK_OUT_1_MUX 356
> -#define TEGRA210_CLK_CLK_OUT_2_MUX 357
> -#define TEGRA210_CLK_CLK_OUT_3_MUX 358
> +/* 356 */
> +/* 357 */
> +/* 358 */
> #define TEGRA210_CLK_DSIA_MUX 359
> #define TEGRA210_CLK_DSIB_MUX 360
> /* 361 */
> diff --git a/include/dt-bindings/clock/tegra30-car.h b/include/dt-bindings/clock/tegra30-car.h
> index 3c90f1535551..20ef2462d9e1 100644
> --- a/include/dt-bindings/clock/tegra30-car.h
> +++ b/include/dt-bindings/clock/tegra30-car.h
> @@ -230,11 +230,11 @@
> #define TEGRA30_CLK_AUDIO3 204
> #define TEGRA30_CLK_AUDIO4 205
> #define TEGRA30_CLK_SPDIF 206
> -#define TEGRA30_CLK_CLK_OUT_1 207 /* (extern1) */
> -#define TEGRA30_CLK_CLK_OUT_2 208 /* (extern2) */
> -#define TEGRA30_CLK_CLK_OUT_3 209 /* (extern3) */
> +/* 207 */
> +/* 208 */
> +/* 209 */
> #define TEGRA30_CLK_SCLK 210
> -#define TEGRA30_CLK_BLINK 211
> +/* 211 */
> #define TEGRA30_CLK_CCLK_G 212
> #define TEGRA30_CLK_CCLK_LP 213
> #define TEGRA30_CLK_TWD 214
> @@ -260,9 +260,9 @@
> /* 297 */
> /* 298 */
> /* 299 */
> -#define TEGRA30_CLK_CLK_OUT_1_MUX 300
> -#define TEGRA30_CLK_CLK_OUT_2_MUX 301
> -#define TEGRA30_CLK_CLK_OUT_3_MUX 302
> +/* 300 */
> +/* 301 */
> +/* 302 */
> #define TEGRA30_CLK_AUDIO0_MUX 303
> #define TEGRA30_CLK_AUDIO1_MUX 304
> #define TEGRA30_CLK_AUDIO2_MUX 305
>

This a device-tree ABI breakage and I'm not sure that it's okay to break
older device-trees (Pixel C Smaug board), maybe some kind of fallback is
needed.

2019-11-27 17:04:14

by Sowjanya Komatineni

[permalink] [raw]
Subject: Re: [PATCH v2 00/11] Move PMC clocks into Tegra PMC driver


On 11/27/19 6:31 AM, Dmitry Osipenko wrote:
> 27.11.2019 07:59, Sowjanya Komatineni пишет:
>> Tegra PMC has clk_out_1, clk_out_2, clk_out_3 and blink controls which
>> are currently registered by Tegra clock driver using clk_regiser_mux and
>> clk_register_gate which performs direct Tegra PMC register access.
>>
>> When Tegra PMC is in secure mode, any access from non-secure world will
>> not go through.
>>
>> This patch series adds these Tegra PMC clocks and blink controls to Tegra
>> PMC driver with PMC as clock provider and removed them from Tegra clock
>> driver. This also adds PMC specific clock id's to use in device tree and
>> removed clock ids of PMC clock from Tegra clock driver.
>>
>> This series also includes patch to update clock provider from tegra_car
>> to pmc in the device tree tegra210-smaug.dts that uses clk_out_2 from PMC.
>>
>> [v2]: Changes between v1 and v2 are
>> - v2 includes patches for adding clk_out_1, clk_out_2, clk_out_3,
>> blink controls to Tegra PMC driver and removing clk-tegra-pmc.
>> - feedback related to pmc clocks in Tegra PMC driver from v1
>> - Removed patches for WB0 PLLM overrides and PLLE IDDQ PMC programming
>> by the clock driver using helper functions from Tegra PMC.
>>
>> Note:
>> To use helper functions from PMC driver, PMC early init need to
>> happen prior to using helper functions and these helper functions are
>> for PLLM Override and PLLE IDDQ programming in PMC during PLLM/PLLE
>> clock registration which happen in clock_init prior to Tegra PMC
>> probe.
>> Moving PLLM/PLLE clocks registration to happen after Tegra PMC
>> impacts other clocks EMC, MC and corresponding tegra_emc_init and
>> tegra_mc_init.
>> This implementation of configuring PMC registers thru helper
>> functions in clock driver needs proper changes across PMC, Clock,
>> EMC and MC inits to have it work across all Tegra platforms.
>>
>> Currently PLLM Override is not enabled in the bootloader so proper
>> patches for this fix will be taken care separately.
> Hello Sowjanya,
>
> Could you please clarify what do you mean by "PLLM Override not enabled
> in bootloader"?
>
> There is T124 Nyan Big Chromebook which is supported in upstream kernel,
> it has PLLM Override set by bootloader. I also have T30 Nexus 7 tablet
> which has the PLLM Override set by bootloader as well. It's not clear to
> me whether this patch series is supposed to break these devices. If the
> breakage is the case here, then I'm afraid you can't postpone supporting
> the PLLM Override and a full-featured implementation is needed.

Hi Dmitry,

Secure boot currently is enabled only on Tegra210 and Tegra210
bootloader doesn't enable PLLM override.

So PLLM override/PLLE IDDQ being in clock driver currently will not
break on any of existing Tegra platforms.

>
> I briefly tried to test this series on T30 and this time it doesn't hang
> on boot, but somehow WiFi MMC card detection is broken. AFAIK, the WiFi
> chip uses the Blink clock source and the clock should be enabled by the
> MMC core because this is how DT part looks like:
>
> brcm_wifi_pwrseq: wifi-pwrseq {
> compatible = "mmc-pwrseq-simple";
> clocks = <&pmc TEGRA_PMC_CLK_BLINK>;
> clock-names = "ext_clock";
> reset-gpios = <&gpio TEGRA_GPIO(D, 3) GPIO_ACTIVE_LOW>;
> post-power-on-delay-ms = <300>;
> power-off-delay-us = <300>;
> };
>
> BTW, I tried this series on a T20 device which also uses the Blink
> clock for WiFi card and it works. So looks like this patchset has some
> problem in regards to the T30 PMC clocks implementation.
>
> [snip]

Blink init state is set to true for both Tegra20 and Tegra30 and all go
through the same blink programming sequence.

Will try to add more debug messages to dump registers and will test
blink through device tree on T30 and will get back...


2019-11-27 17:08:25

by Sowjanya Komatineni

[permalink] [raw]
Subject: Re: [PATCH v2 06/11] dt-bindings: clock: tegra: Remove pmc clock ids from clock dt-bindings


On 11/27/19 6:32 AM, Dmitry Osipenko wrote:
> 27.11.2019 07:59, Sowjanya Komatineni пишет:
>> clk_out_1, clk_out_2, clk_out_3, blink are part of Tegra pmc clocks.
>>
>> This patch removes ids for these clocks from Tegra clock dt-bindings.
>>
>> Signed-off-by: Sowjanya Komatineni <[email protected]>
>> ---
>> include/dt-bindings/clock/tegra114-car.h | 14 +++++++-------
>> include/dt-bindings/clock/tegra124-car-common.h | 14 +++++++-------
>> include/dt-bindings/clock/tegra20-car.h | 2 +-
>> include/dt-bindings/clock/tegra210-car.h | 14 +++++++-------
>> include/dt-bindings/clock/tegra30-car.h | 14 +++++++-------
>> 5 files changed, 29 insertions(+), 29 deletions(-)
>>
>> diff --git a/include/dt-bindings/clock/tegra114-car.h b/include/dt-bindings/clock/tegra114-car.h
>> index bb5c2c999c05..9175cd0571b5 100644
>> --- a/include/dt-bindings/clock/tegra114-car.h
>> +++ b/include/dt-bindings/clock/tegra114-car.h
>> @@ -270,10 +270,10 @@
>> #define TEGRA114_CLK_AUDIO3 242
>> #define TEGRA114_CLK_AUDIO4 243
>> #define TEGRA114_CLK_SPDIF 244
>> -#define TEGRA114_CLK_CLK_OUT_1 245
>> -#define TEGRA114_CLK_CLK_OUT_2 246
>> -#define TEGRA114_CLK_CLK_OUT_3 247
>> -#define TEGRA114_CLK_BLINK 248
>> +/* 245 */
>> +/* 246 */
>> +/* 247 */
>> +/* 248 */
>> /* 249 */
>> /* 250 */
>> /* 251 */
>> @@ -333,9 +333,9 @@
>> #define TEGRA114_CLK_AUDIO3_MUX 303
>> #define TEGRA114_CLK_AUDIO4_MUX 304
>> #define TEGRA114_CLK_SPDIF_MUX 305
>> -#define TEGRA114_CLK_CLK_OUT_1_MUX 306
>> -#define TEGRA114_CLK_CLK_OUT_2_MUX 307
>> -#define TEGRA114_CLK_CLK_OUT_3_MUX 308
>> +/* 306 */
>> +/* 307 */
>> +/* 308 */
>> #define TEGRA114_CLK_DSIA_MUX 309
>> #define TEGRA114_CLK_DSIB_MUX 310
>> #define TEGRA114_CLK_XUSB_SS_DIV2 311
>> diff --git a/include/dt-bindings/clock/tegra124-car-common.h b/include/dt-bindings/clock/tegra124-car-common.h
>> index 0c4f5be0a742..90a0c5e7eb5f 100644
>> --- a/include/dt-bindings/clock/tegra124-car-common.h
>> +++ b/include/dt-bindings/clock/tegra124-car-common.h
>> @@ -269,10 +269,10 @@
>> #define TEGRA124_CLK_AUDIO3 242
>> #define TEGRA124_CLK_AUDIO4 243
>> #define TEGRA124_CLK_SPDIF 244
>> -#define TEGRA124_CLK_CLK_OUT_1 245
>> -#define TEGRA124_CLK_CLK_OUT_2 246
>> -#define TEGRA124_CLK_CLK_OUT_3 247
>> -#define TEGRA124_CLK_BLINK 248
>> +/* 245 */
>> +/* 246 */
>> +/* 247 */
>> +/* 248 */
>> /* 249 */
>> /* 250 */
>> /* 251 */
>> @@ -332,9 +332,9 @@
>> #define TEGRA124_CLK_AUDIO3_MUX 303
>> #define TEGRA124_CLK_AUDIO4_MUX 304
>> #define TEGRA124_CLK_SPDIF_MUX 305
>> -#define TEGRA124_CLK_CLK_OUT_1_MUX 306
>> -#define TEGRA124_CLK_CLK_OUT_2_MUX 307
>> -#define TEGRA124_CLK_CLK_OUT_3_MUX 308
>> +/* 306 */
>> +/* 307 */
>> +/* 308 */
>> /* 309 */
>> /* 310 */
>> #define TEGRA124_CLK_SOR0_LVDS 311 /* deprecated */
>> diff --git a/include/dt-bindings/clock/tegra20-car.h b/include/dt-bindings/clock/tegra20-car.h
>> index b21a0eb32921..fe541f627965 100644
>> --- a/include/dt-bindings/clock/tegra20-car.h
>> +++ b/include/dt-bindings/clock/tegra20-car.h
>> @@ -131,7 +131,7 @@
>> #define TEGRA20_CLK_CCLK 108
>> #define TEGRA20_CLK_HCLK 109
>> #define TEGRA20_CLK_PCLK 110
>> -#define TEGRA20_CLK_BLINK 111
>> +/* 111 */
>> #define TEGRA20_CLK_PLL_A 112
>> #define TEGRA20_CLK_PLL_A_OUT0 113
>> #define TEGRA20_CLK_PLL_C 114
>> diff --git a/include/dt-bindings/clock/tegra210-car.h b/include/dt-bindings/clock/tegra210-car.h
>> index 44f60623f99b..a3d8d3e75728 100644
>> --- a/include/dt-bindings/clock/tegra210-car.h
>> +++ b/include/dt-bindings/clock/tegra210-car.h
>> @@ -304,10 +304,10 @@
>> #define TEGRA210_CLK_AUDIO3 274
>> #define TEGRA210_CLK_AUDIO4 275
>> #define TEGRA210_CLK_SPDIF 276
>> -#define TEGRA210_CLK_CLK_OUT_1 277
>> -#define TEGRA210_CLK_CLK_OUT_2 278
>> -#define TEGRA210_CLK_CLK_OUT_3 279
>> -#define TEGRA210_CLK_BLINK 280
>> +/* 277 */
>> +/* 278 */
>> +/* 279 */
>> +/* 280 */
>> #define TEGRA210_CLK_SOR0_LVDS 281 /* deprecated */
>> #define TEGRA210_CLK_SOR0_OUT 281
>> #define TEGRA210_CLK_SOR1_OUT 282
>> @@ -386,9 +386,9 @@
>> #define TEGRA210_CLK_AUDIO3_MUX 353
>> #define TEGRA210_CLK_AUDIO4_MUX 354
>> #define TEGRA210_CLK_SPDIF_MUX 355
>> -#define TEGRA210_CLK_CLK_OUT_1_MUX 356
>> -#define TEGRA210_CLK_CLK_OUT_2_MUX 357
>> -#define TEGRA210_CLK_CLK_OUT_3_MUX 358
>> +/* 356 */
>> +/* 357 */
>> +/* 358 */
>> #define TEGRA210_CLK_DSIA_MUX 359
>> #define TEGRA210_CLK_DSIB_MUX 360
>> /* 361 */
>> diff --git a/include/dt-bindings/clock/tegra30-car.h b/include/dt-bindings/clock/tegra30-car.h
>> index 3c90f1535551..20ef2462d9e1 100644
>> --- a/include/dt-bindings/clock/tegra30-car.h
>> +++ b/include/dt-bindings/clock/tegra30-car.h
>> @@ -230,11 +230,11 @@
>> #define TEGRA30_CLK_AUDIO3 204
>> #define TEGRA30_CLK_AUDIO4 205
>> #define TEGRA30_CLK_SPDIF 206
>> -#define TEGRA30_CLK_CLK_OUT_1 207 /* (extern1) */
>> -#define TEGRA30_CLK_CLK_OUT_2 208 /* (extern2) */
>> -#define TEGRA30_CLK_CLK_OUT_3 209 /* (extern3) */
>> +/* 207 */
>> +/* 208 */
>> +/* 209 */
>> #define TEGRA30_CLK_SCLK 210
>> -#define TEGRA30_CLK_BLINK 211
>> +/* 211 */
>> #define TEGRA30_CLK_CCLK_G 212
>> #define TEGRA30_CLK_CCLK_LP 213
>> #define TEGRA30_CLK_TWD 214
>> @@ -260,9 +260,9 @@
>> /* 297 */
>> /* 298 */
>> /* 299 */
>> -#define TEGRA30_CLK_CLK_OUT_1_MUX 300
>> -#define TEGRA30_CLK_CLK_OUT_2_MUX 301
>> -#define TEGRA30_CLK_CLK_OUT_3_MUX 302
>> +/* 300 */
>> +/* 301 */
>> +/* 302 */
>> #define TEGRA30_CLK_AUDIO0_MUX 303
>> #define TEGRA30_CLK_AUDIO1_MUX 304
>> #define TEGRA30_CLK_AUDIO2_MUX 305
>>
> This a device-tree ABI breakage and I'm not sure that it's okay to break
> older device-trees (Pixel C Smaug board), maybe some kind of fallback is
> needed.

As provider itself changed from tegra_car to pmc, all platform
device-trees using CLK_OUT_1/2/3 ids from Tegra car should be moved to
PMC and use PMC clock ids.

Based on upstream device tree search, I only see samsung t210 device
tree currently using CLK_OUT_2. So updated that as part of this series.

2019-11-27 21:43:09

by Sowjanya Komatineni

[permalink] [raw]
Subject: Re: [PATCH v2 00/11] Move PMC clocks into Tegra PMC driver


On 11/27/19 9:02 AM, Sowjanya Komatineni wrote:
>
> On 11/27/19 6:31 AM, Dmitry Osipenko wrote:
>> 27.11.2019 07:59, Sowjanya Komatineni пишет:
>>> Tegra PMC has clk_out_1, clk_out_2, clk_out_3 and blink controls which
>>> are currently registered by Tegra clock driver using clk_regiser_mux
>>> and
>>> clk_register_gate which performs direct Tegra PMC register access.
>>>
>>> When Tegra PMC is in secure mode, any access from non-secure world will
>>> not go through.
>>>
>>> This patch series adds these Tegra PMC clocks and blink controls to
>>> Tegra
>>> PMC driver with PMC as clock provider and removed them from Tegra clock
>>> driver. This also adds PMC specific clock id's to use in device tree
>>> and
>>> removed clock ids of PMC clock from Tegra clock driver.
>>>
>>> This series also includes patch to update clock provider from tegra_car
>>> to pmc in the device tree tegra210-smaug.dts that uses clk_out_2
>>> from PMC.
>>>
>>> [v2]:    Changes between v1 and v2 are
>>>     - v2 includes patches for adding clk_out_1, clk_out_2, clk_out_3,
>>>       blink controls to Tegra PMC driver and removing clk-tegra-pmc.
>>>     - feedback related to pmc clocks in Tegra PMC driver from v1
>>>     - Removed patches for WB0 PLLM overrides and PLLE IDDQ PMC
>>> programming
>>>       by the clock driver using helper functions from Tegra PMC.
>>>
>>>         Note:
>>>       To use helper functions from PMC driver, PMC early init need to
>>>       happen prior to using helper functions and these helper
>>> functions are
>>>       for PLLM Override and PLLE IDDQ programming in PMC during
>>> PLLM/PLLE
>>>       clock registration which happen in clock_init prior to Tegra PMC
>>>       probe.
>>>       Moving PLLM/PLLE clocks registration to happen after Tegra PMC
>>>       impacts other clocks EMC, MC and corresponding tegra_emc_init and
>>>       tegra_mc_init.
>>>       This implementation of configuring PMC registers thru helper
>>>       functions in clock driver needs proper changes across PMC, Clock,
>>>       EMC and MC inits to have it work across all Tegra platforms.
>>>
>>>       Currently PLLM Override is not enabled in the bootloader so
>>> proper
>>>       patches for this fix will be taken care separately.
>> Hello Sowjanya,
>>
>> Could you please clarify what do you mean by "PLLM Override not enabled
>> in bootloader"?
>>
>> There is T124 Nyan Big Chromebook which is supported in upstream kernel,
>> it has PLLM Override set by bootloader. I also have T30 Nexus 7 tablet
>> which has the PLLM Override set by bootloader as well. It's not clear to
>> me whether this patch series is supposed to break these devices. If the
>> breakage is the case here, then I'm afraid you can't postpone supporting
>> the PLLM Override and a full-featured implementation is needed.
>
> Hi Dmitry,
>
> Secure boot currently is enabled only on Tegra210 and Tegra210
> bootloader doesn't enable PLLM override.
>
> So PLLM override/PLLE IDDQ being in clock driver currently will not
> break on any of existing Tegra platforms.
>
>>
>> I briefly tried to test this series on T30 and this time it doesn't hang
>> on boot, but somehow WiFi MMC card detection is broken. AFAIK, the WiFi
>> chip uses the Blink clock source and the clock should be enabled by the
>> MMC core because this is how DT part looks like:
>>
>> brcm_wifi_pwrseq: wifi-pwrseq {
>>     compatible = "mmc-pwrseq-simple";
>>     clocks = <&pmc TEGRA_PMC_CLK_BLINK>;
>>     clock-names = "ext_clock";
>>     reset-gpios =  <&gpio TEGRA_GPIO(D, 3) GPIO_ACTIVE_LOW>;
>>     post-power-on-delay-ms = <300>;
>>     power-off-delay-us = <300>;
>> };
>>
>> BTW, I  tried this series on a T20 device which also uses the Blink
>> clock for WiFi card and it works. So looks like this patchset has some
>> problem in regards to the T30 PMC clocks implementation.
>>
>> [snip]
>
> Blink init state is set to true for both Tegra20 and Tegra30 and all
> go through the same blink programming sequence.
>
> Will try to add more debug messages to dump registers and will test
> blink through device tree on T30 and will get back...
>
>
define value for BLINK uses BIT macro instead of just position. Will fix
this in v3.

2019-11-28 12:20:51

by Thierry Reding

[permalink] [raw]
Subject: Re: [PATCH v2 06/11] dt-bindings: clock: tegra: Remove pmc clock ids from clock dt-bindings

On Wed, Nov 27, 2019 at 05:32:30PM +0300, Dmitry Osipenko wrote:
> 27.11.2019 07:59, Sowjanya Komatineni пишет:
> > clk_out_1, clk_out_2, clk_out_3, blink are part of Tegra pmc clocks.
> >
> > This patch removes ids for these clocks from Tegra clock dt-bindings.
> >
> > Signed-off-by: Sowjanya Komatineni <[email protected]>
> > ---
> > include/dt-bindings/clock/tegra114-car.h | 14 +++++++-------
> > include/dt-bindings/clock/tegra124-car-common.h | 14 +++++++-------
> > include/dt-bindings/clock/tegra20-car.h | 2 +-
> > include/dt-bindings/clock/tegra210-car.h | 14 +++++++-------
> > include/dt-bindings/clock/tegra30-car.h | 14 +++++++-------
> > 5 files changed, 29 insertions(+), 29 deletions(-)
> >
> > diff --git a/include/dt-bindings/clock/tegra114-car.h b/include/dt-bindings/clock/tegra114-car.h
> > index bb5c2c999c05..9175cd0571b5 100644
> > --- a/include/dt-bindings/clock/tegra114-car.h
> > +++ b/include/dt-bindings/clock/tegra114-car.h
> > @@ -270,10 +270,10 @@
> > #define TEGRA114_CLK_AUDIO3 242
> > #define TEGRA114_CLK_AUDIO4 243
> > #define TEGRA114_CLK_SPDIF 244
> > -#define TEGRA114_CLK_CLK_OUT_1 245
> > -#define TEGRA114_CLK_CLK_OUT_2 246
> > -#define TEGRA114_CLK_CLK_OUT_3 247
> > -#define TEGRA114_CLK_BLINK 248
> > +/* 245 */
> > +/* 246 */
> > +/* 247 */
> > +/* 248 */
> > /* 249 */
> > /* 250 */
> > /* 251 */
> > @@ -333,9 +333,9 @@
> > #define TEGRA114_CLK_AUDIO3_MUX 303
> > #define TEGRA114_CLK_AUDIO4_MUX 304
> > #define TEGRA114_CLK_SPDIF_MUX 305
> > -#define TEGRA114_CLK_CLK_OUT_1_MUX 306
> > -#define TEGRA114_CLK_CLK_OUT_2_MUX 307
> > -#define TEGRA114_CLK_CLK_OUT_3_MUX 308
> > +/* 306 */
> > +/* 307 */
> > +/* 308 */
> > #define TEGRA114_CLK_DSIA_MUX 309
> > #define TEGRA114_CLK_DSIB_MUX 310
> > #define TEGRA114_CLK_XUSB_SS_DIV2 311
> > diff --git a/include/dt-bindings/clock/tegra124-car-common.h b/include/dt-bindings/clock/tegra124-car-common.h
> > index 0c4f5be0a742..90a0c5e7eb5f 100644
> > --- a/include/dt-bindings/clock/tegra124-car-common.h
> > +++ b/include/dt-bindings/clock/tegra124-car-common.h
> > @@ -269,10 +269,10 @@
> > #define TEGRA124_CLK_AUDIO3 242
> > #define TEGRA124_CLK_AUDIO4 243
> > #define TEGRA124_CLK_SPDIF 244
> > -#define TEGRA124_CLK_CLK_OUT_1 245
> > -#define TEGRA124_CLK_CLK_OUT_2 246
> > -#define TEGRA124_CLK_CLK_OUT_3 247
> > -#define TEGRA124_CLK_BLINK 248
> > +/* 245 */
> > +/* 246 */
> > +/* 247 */
> > +/* 248 */
> > /* 249 */
> > /* 250 */
> > /* 251 */
> > @@ -332,9 +332,9 @@
> > #define TEGRA124_CLK_AUDIO3_MUX 303
> > #define TEGRA124_CLK_AUDIO4_MUX 304
> > #define TEGRA124_CLK_SPDIF_MUX 305
> > -#define TEGRA124_CLK_CLK_OUT_1_MUX 306
> > -#define TEGRA124_CLK_CLK_OUT_2_MUX 307
> > -#define TEGRA124_CLK_CLK_OUT_3_MUX 308
> > +/* 306 */
> > +/* 307 */
> > +/* 308 */
> > /* 309 */
> > /* 310 */
> > #define TEGRA124_CLK_SOR0_LVDS 311 /* deprecated */
> > diff --git a/include/dt-bindings/clock/tegra20-car.h b/include/dt-bindings/clock/tegra20-car.h
> > index b21a0eb32921..fe541f627965 100644
> > --- a/include/dt-bindings/clock/tegra20-car.h
> > +++ b/include/dt-bindings/clock/tegra20-car.h
> > @@ -131,7 +131,7 @@
> > #define TEGRA20_CLK_CCLK 108
> > #define TEGRA20_CLK_HCLK 109
> > #define TEGRA20_CLK_PCLK 110
> > -#define TEGRA20_CLK_BLINK 111
> > +/* 111 */
> > #define TEGRA20_CLK_PLL_A 112
> > #define TEGRA20_CLK_PLL_A_OUT0 113
> > #define TEGRA20_CLK_PLL_C 114
> > diff --git a/include/dt-bindings/clock/tegra210-car.h b/include/dt-bindings/clock/tegra210-car.h
> > index 44f60623f99b..a3d8d3e75728 100644
> > --- a/include/dt-bindings/clock/tegra210-car.h
> > +++ b/include/dt-bindings/clock/tegra210-car.h
> > @@ -304,10 +304,10 @@
> > #define TEGRA210_CLK_AUDIO3 274
> > #define TEGRA210_CLK_AUDIO4 275
> > #define TEGRA210_CLK_SPDIF 276
> > -#define TEGRA210_CLK_CLK_OUT_1 277
> > -#define TEGRA210_CLK_CLK_OUT_2 278
> > -#define TEGRA210_CLK_CLK_OUT_3 279
> > -#define TEGRA210_CLK_BLINK 280
> > +/* 277 */
> > +/* 278 */
> > +/* 279 */
> > +/* 280 */
> > #define TEGRA210_CLK_SOR0_LVDS 281 /* deprecated */
> > #define TEGRA210_CLK_SOR0_OUT 281
> > #define TEGRA210_CLK_SOR1_OUT 282
> > @@ -386,9 +386,9 @@
> > #define TEGRA210_CLK_AUDIO3_MUX 353
> > #define TEGRA210_CLK_AUDIO4_MUX 354
> > #define TEGRA210_CLK_SPDIF_MUX 355
> > -#define TEGRA210_CLK_CLK_OUT_1_MUX 356
> > -#define TEGRA210_CLK_CLK_OUT_2_MUX 357
> > -#define TEGRA210_CLK_CLK_OUT_3_MUX 358
> > +/* 356 */
> > +/* 357 */
> > +/* 358 */
> > #define TEGRA210_CLK_DSIA_MUX 359
> > #define TEGRA210_CLK_DSIB_MUX 360
> > /* 361 */
> > diff --git a/include/dt-bindings/clock/tegra30-car.h b/include/dt-bindings/clock/tegra30-car.h
> > index 3c90f1535551..20ef2462d9e1 100644
> > --- a/include/dt-bindings/clock/tegra30-car.h
> > +++ b/include/dt-bindings/clock/tegra30-car.h
> > @@ -230,11 +230,11 @@
> > #define TEGRA30_CLK_AUDIO3 204
> > #define TEGRA30_CLK_AUDIO4 205
> > #define TEGRA30_CLK_SPDIF 206
> > -#define TEGRA30_CLK_CLK_OUT_1 207 /* (extern1) */
> > -#define TEGRA30_CLK_CLK_OUT_2 208 /* (extern2) */
> > -#define TEGRA30_CLK_CLK_OUT_3 209 /* (extern3) */
> > +/* 207 */
> > +/* 208 */
> > +/* 209 */
> > #define TEGRA30_CLK_SCLK 210
> > -#define TEGRA30_CLK_BLINK 211
> > +/* 211 */
> > #define TEGRA30_CLK_CCLK_G 212
> > #define TEGRA30_CLK_CCLK_LP 213
> > #define TEGRA30_CLK_TWD 214
> > @@ -260,9 +260,9 @@
> > /* 297 */
> > /* 298 */
> > /* 299 */
> > -#define TEGRA30_CLK_CLK_OUT_1_MUX 300
> > -#define TEGRA30_CLK_CLK_OUT_2_MUX 301
> > -#define TEGRA30_CLK_CLK_OUT_3_MUX 302
> > +/* 300 */
> > +/* 301 */
> > +/* 302 */
> > #define TEGRA30_CLK_AUDIO0_MUX 303
> > #define TEGRA30_CLK_AUDIO1_MUX 304
> > #define TEGRA30_CLK_AUDIO2_MUX 305
> >
>
> This a device-tree ABI breakage and I'm not sure that it's okay to break
> older device-trees (Pixel C Smaug board), maybe some kind of fallback is
> needed.

The Smaug support was never really official. I don't think anybody uses
upstream on it "for real" because there's a very limited set of features
that we do support. My understanding is that there is a community around
Pixel C that runs their own forks with some more features and they have
occasionally upstreamed bits and pieces of that.

So given how far behind we are with Smaug, I don't think breaking ABI
stability is really a problem in this case.

Jon, we used to have a Smaug system in our internal upstream testing,
but that's no longer the case. If we ever were to reinstate that
testing, updating the DTB shouldn't be a problem, right? My recollection
is that updating the DTB was always done hand in hand with the kernel
update.

Thierry


Attachments:
(No filename) (6.69 kB)
signature.asc (849.00 B)
Download all attachments

2019-11-28 12:28:41

by Thierry Reding

[permalink] [raw]
Subject: Re: [PATCH v2 00/11] Move PMC clocks into Tegra PMC driver

On Wed, Nov 27, 2019 at 05:31:34PM +0300, Dmitry Osipenko wrote:
> 27.11.2019 07:59, Sowjanya Komatineni пишет:
> > Tegra PMC has clk_out_1, clk_out_2, clk_out_3 and blink controls which
> > are currently registered by Tegra clock driver using clk_regiser_mux and
> > clk_register_gate which performs direct Tegra PMC register access.
> >
> > When Tegra PMC is in secure mode, any access from non-secure world will
> > not go through.
> >
> > This patch series adds these Tegra PMC clocks and blink controls to Tegra
> > PMC driver with PMC as clock provider and removed them from Tegra clock
> > driver. This also adds PMC specific clock id's to use in device tree and
> > removed clock ids of PMC clock from Tegra clock driver.
> >
> > This series also includes patch to update clock provider from tegra_car
> > to pmc in the device tree tegra210-smaug.dts that uses clk_out_2 from PMC.
> >
> > [v2]: Changes between v1 and v2 are
> > - v2 includes patches for adding clk_out_1, clk_out_2, clk_out_3,
> > blink controls to Tegra PMC driver and removing clk-tegra-pmc.
> > - feedback related to pmc clocks in Tegra PMC driver from v1
> > - Removed patches for WB0 PLLM overrides and PLLE IDDQ PMC programming
> > by the clock driver using helper functions from Tegra PMC.
> >
> > Note:
> > To use helper functions from PMC driver, PMC early init need to
> > happen prior to using helper functions and these helper functions are
> > for PLLM Override and PLLE IDDQ programming in PMC during PLLM/PLLE
> > clock registration which happen in clock_init prior to Tegra PMC
> > probe.
> > Moving PLLM/PLLE clocks registration to happen after Tegra PMC
> > impacts other clocks EMC, MC and corresponding tegra_emc_init and
> > tegra_mc_init.
> > This implementation of configuring PMC registers thru helper
> > functions in clock driver needs proper changes across PMC, Clock,
> > EMC and MC inits to have it work across all Tegra platforms.
> >
> > Currently PLLM Override is not enabled in the bootloader so proper
> > patches for this fix will be taken care separately.
>
> Hello Sowjanya,
>
> Could you please clarify what do you mean by "PLLM Override not enabled
> in bootloader"?
>
> There is T124 Nyan Big Chromebook which is supported in upstream kernel,
> it has PLLM Override set by bootloader. I also have T30 Nexus 7 tablet
> which has the PLLM Override set by bootloader as well. It's not clear to
> me whether this patch series is supposed to break these devices. If the
> breakage is the case here, then I'm afraid you can't postpone supporting
> the PLLM Override and a full-featured implementation is needed.

For some more background on why we chose to take this shortcut for now:
Sowjanya was looking at the full-featured implementation and that ended
up being a can of worms. The problem is that there are various inter-
dependencies between the PLLM override and the MC/EMC clocks.

Unfortunately we depend a lot on the explicit ordering of driver probe,
especially during early boot, so this started to get very complicated,
very quickly.

The bottom line was basically that we would need to move a whole bunch
of clocks to register at a very late point in time and support deferred
probe throughout in order to make it all work together nicely. Sowjanya
had a crack at that, and while the system ended up booting, there were a
number of errors from the MC and IOMMU drivers.

At the end, we decided to take a look at that separately because, as was
mentioned earlier, the PLLM override is not used on platforms where the
PMC is locked down, so the existing PLLM override code is going to
continue to work fine on the platforms where it's currently used.

Thierry

> I briefly tried to test this series on T30 and this time it doesn't hang
> on boot, but somehow WiFi MMC card detection is broken. AFAIK, the WiFi
> chip uses the Blink clock source and the clock should be enabled by the
> MMC core because this is how DT part looks like:
>
> brcm_wifi_pwrseq: wifi-pwrseq {
> compatible = "mmc-pwrseq-simple";
> clocks = <&pmc TEGRA_PMC_CLK_BLINK>;
> clock-names = "ext_clock";
> reset-gpios = <&gpio TEGRA_GPIO(D, 3) GPIO_ACTIVE_LOW>;
> post-power-on-delay-ms = <300>;
> power-off-delay-us = <300>;
> };
>
> BTW, I tried this series on a T20 device which also uses the Blink
> clock for WiFi card and it works. So looks like this patchset has some
> problem in regards to the T30 PMC clocks implementation.
>
> [snip]


Attachments:
(No filename) (4.50 kB)
signature.asc (849.00 B)
Download all attachments

2019-11-28 13:08:09

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH v2 00/11] Move PMC clocks into Tegra PMC driver

28.11.2019 00:38, Sowjanya Komatineni пишет:
>
> On 11/27/19 9:02 AM, Sowjanya Komatineni wrote:
>>
>> On 11/27/19 6:31 AM, Dmitry Osipenko wrote:
>>> 27.11.2019 07:59, Sowjanya Komatineni пишет:
>>>> Tegra PMC has clk_out_1, clk_out_2, clk_out_3 and blink controls which
>>>> are currently registered by Tegra clock driver using clk_regiser_mux
>>>> and
>>>> clk_register_gate which performs direct Tegra PMC register access.
>>>>
>>>> When Tegra PMC is in secure mode, any access from non-secure world will
>>>> not go through.
>>>>
>>>> This patch series adds these Tegra PMC clocks and blink controls to
>>>> Tegra
>>>> PMC driver with PMC as clock provider and removed them from Tegra clock
>>>> driver. This also adds PMC specific clock id's to use in device tree
>>>> and
>>>> removed clock ids of PMC clock from Tegra clock driver.
>>>>
>>>> This series also includes patch to update clock provider from tegra_car
>>>> to pmc in the device tree tegra210-smaug.dts that uses clk_out_2
>>>> from PMC.
>>>>
>>>> [v2]:    Changes between v1 and v2 are
>>>>     - v2 includes patches for adding clk_out_1, clk_out_2, clk_out_3,
>>>>       blink controls to Tegra PMC driver and removing clk-tegra-pmc.
>>>>     - feedback related to pmc clocks in Tegra PMC driver from v1
>>>>     - Removed patches for WB0 PLLM overrides and PLLE IDDQ PMC
>>>> programming
>>>>       by the clock driver using helper functions from Tegra PMC.
>>>>
>>>>         Note:
>>>>       To use helper functions from PMC driver, PMC early init need to
>>>>       happen prior to using helper functions and these helper
>>>> functions are
>>>>       for PLLM Override and PLLE IDDQ programming in PMC during
>>>> PLLM/PLLE
>>>>       clock registration which happen in clock_init prior to Tegra PMC
>>>>       probe.
>>>>       Moving PLLM/PLLE clocks registration to happen after Tegra PMC
>>>>       impacts other clocks EMC, MC and corresponding tegra_emc_init and
>>>>       tegra_mc_init.
>>>>       This implementation of configuring PMC registers thru helper
>>>>       functions in clock driver needs proper changes across PMC, Clock,
>>>>       EMC and MC inits to have it work across all Tegra platforms.
>>>>
>>>>       Currently PLLM Override is not enabled in the bootloader so
>>>> proper
>>>>       patches for this fix will be taken care separately.
>>> Hello Sowjanya,
>>>
>>> Could you please clarify what do you mean by "PLLM Override not enabled
>>> in bootloader"?
>>>
>>> There is T124 Nyan Big Chromebook which is supported in upstream kernel,
>>> it has PLLM Override set by bootloader. I also have T30 Nexus 7 tablet
>>> which has the PLLM Override set by bootloader as well. It's not clear to
>>> me whether this patch series is supposed to break these devices. If the
>>> breakage is the case here, then I'm afraid you can't postpone supporting
>>> the PLLM Override and a full-featured implementation is needed.
>>
>> Hi Dmitry,
>>
>> Secure boot currently is enabled only on Tegra210 and Tegra210
>> bootloader doesn't enable PLLM override.
>>
>> So PLLM override/PLLE IDDQ being in clock driver currently will not
>> break on any of existing Tegra platforms.
>>
>>>
>>> I briefly tried to test this series on T30 and this time it doesn't hang
>>> on boot, but somehow WiFi MMC card detection is broken. AFAIK, the WiFi
>>> chip uses the Blink clock source and the clock should be enabled by the
>>> MMC core because this is how DT part looks like:
>>>
>>> brcm_wifi_pwrseq: wifi-pwrseq {
>>>     compatible = "mmc-pwrseq-simple";
>>>     clocks = <&pmc TEGRA_PMC_CLK_BLINK>;
>>>     clock-names = "ext_clock";
>>>     reset-gpios =  <&gpio TEGRA_GPIO(D, 3) GPIO_ACTIVE_LOW>;
>>>     post-power-on-delay-ms = <300>;
>>>     power-off-delay-us = <300>;
>>> };
>>>
>>> BTW, I  tried this series on a T20 device which also uses the Blink
>>> clock for WiFi card and it works. So looks like this patchset has some
>>> problem in regards to the T30 PMC clocks implementation.
>>>
>>> [snip]
>>
>> Blink init state is set to true for both Tegra20 and Tegra30 and all
>> go through the same blink programming sequence.
>>
>> Will try to add more debug messages to dump registers and will test
>> blink through device tree on T30 and will get back...
>>
>>
> define value for BLINK uses BIT macro instead of just position. Will fix
> this in v3.

Thanks, will try v3 once it will be ready.

I took a look through the T20 board's schematics and seems it doesn't
use the Blink clock for the WiFi, instead it uses 32k source directly
from PMU. While T30 board schematics tells that 32k comes out from the
Tegra chip.

BTW, I'm curious what's the reason for having Blink clock always-ON on
T20/30, any insights? Looks like it's just some relic from old clk
driver and it should be safe to drop the always-ON.

2019-11-28 13:10:17

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH v2 06/11] dt-bindings: clock: tegra: Remove pmc clock ids from clock dt-bindings

28.11.2019 15:18, Thierry Reding пишет:
> On Wed, Nov 27, 2019 at 05:32:30PM +0300, Dmitry Osipenko wrote:
>> 27.11.2019 07:59, Sowjanya Komatineni пишет:
>>> clk_out_1, clk_out_2, clk_out_3, blink are part of Tegra pmc clocks.
>>>
>>> This patch removes ids for these clocks from Tegra clock dt-bindings.
>>>
>>> Signed-off-by: Sowjanya Komatineni <[email protected]>
>>> ---
>>> include/dt-bindings/clock/tegra114-car.h | 14 +++++++-------
>>> include/dt-bindings/clock/tegra124-car-common.h | 14 +++++++-------
>>> include/dt-bindings/clock/tegra20-car.h | 2 +-
>>> include/dt-bindings/clock/tegra210-car.h | 14 +++++++-------
>>> include/dt-bindings/clock/tegra30-car.h | 14 +++++++-------
>>> 5 files changed, 29 insertions(+), 29 deletions(-)
>>>
>>> diff --git a/include/dt-bindings/clock/tegra114-car.h b/include/dt-bindings/clock/tegra114-car.h
>>> index bb5c2c999c05..9175cd0571b5 100644
>>> --- a/include/dt-bindings/clock/tegra114-car.h
>>> +++ b/include/dt-bindings/clock/tegra114-car.h
>>> @@ -270,10 +270,10 @@
>>> #define TEGRA114_CLK_AUDIO3 242
>>> #define TEGRA114_CLK_AUDIO4 243
>>> #define TEGRA114_CLK_SPDIF 244
>>> -#define TEGRA114_CLK_CLK_OUT_1 245
>>> -#define TEGRA114_CLK_CLK_OUT_2 246
>>> -#define TEGRA114_CLK_CLK_OUT_3 247
>>> -#define TEGRA114_CLK_BLINK 248
>>> +/* 245 */
>>> +/* 246 */
>>> +/* 247 */
>>> +/* 248 */
>>> /* 249 */
>>> /* 250 */
>>> /* 251 */
>>> @@ -333,9 +333,9 @@
>>> #define TEGRA114_CLK_AUDIO3_MUX 303
>>> #define TEGRA114_CLK_AUDIO4_MUX 304
>>> #define TEGRA114_CLK_SPDIF_MUX 305
>>> -#define TEGRA114_CLK_CLK_OUT_1_MUX 306
>>> -#define TEGRA114_CLK_CLK_OUT_2_MUX 307
>>> -#define TEGRA114_CLK_CLK_OUT_3_MUX 308
>>> +/* 306 */
>>> +/* 307 */
>>> +/* 308 */
>>> #define TEGRA114_CLK_DSIA_MUX 309
>>> #define TEGRA114_CLK_DSIB_MUX 310
>>> #define TEGRA114_CLK_XUSB_SS_DIV2 311
>>> diff --git a/include/dt-bindings/clock/tegra124-car-common.h b/include/dt-bindings/clock/tegra124-car-common.h
>>> index 0c4f5be0a742..90a0c5e7eb5f 100644
>>> --- a/include/dt-bindings/clock/tegra124-car-common.h
>>> +++ b/include/dt-bindings/clock/tegra124-car-common.h
>>> @@ -269,10 +269,10 @@
>>> #define TEGRA124_CLK_AUDIO3 242
>>> #define TEGRA124_CLK_AUDIO4 243
>>> #define TEGRA124_CLK_SPDIF 244
>>> -#define TEGRA124_CLK_CLK_OUT_1 245
>>> -#define TEGRA124_CLK_CLK_OUT_2 246
>>> -#define TEGRA124_CLK_CLK_OUT_3 247
>>> -#define TEGRA124_CLK_BLINK 248
>>> +/* 245 */
>>> +/* 246 */
>>> +/* 247 */
>>> +/* 248 */
>>> /* 249 */
>>> /* 250 */
>>> /* 251 */
>>> @@ -332,9 +332,9 @@
>>> #define TEGRA124_CLK_AUDIO3_MUX 303
>>> #define TEGRA124_CLK_AUDIO4_MUX 304
>>> #define TEGRA124_CLK_SPDIF_MUX 305
>>> -#define TEGRA124_CLK_CLK_OUT_1_MUX 306
>>> -#define TEGRA124_CLK_CLK_OUT_2_MUX 307
>>> -#define TEGRA124_CLK_CLK_OUT_3_MUX 308
>>> +/* 306 */
>>> +/* 307 */
>>> +/* 308 */
>>> /* 309 */
>>> /* 310 */
>>> #define TEGRA124_CLK_SOR0_LVDS 311 /* deprecated */
>>> diff --git a/include/dt-bindings/clock/tegra20-car.h b/include/dt-bindings/clock/tegra20-car.h
>>> index b21a0eb32921..fe541f627965 100644
>>> --- a/include/dt-bindings/clock/tegra20-car.h
>>> +++ b/include/dt-bindings/clock/tegra20-car.h
>>> @@ -131,7 +131,7 @@
>>> #define TEGRA20_CLK_CCLK 108
>>> #define TEGRA20_CLK_HCLK 109
>>> #define TEGRA20_CLK_PCLK 110
>>> -#define TEGRA20_CLK_BLINK 111
>>> +/* 111 */
>>> #define TEGRA20_CLK_PLL_A 112
>>> #define TEGRA20_CLK_PLL_A_OUT0 113
>>> #define TEGRA20_CLK_PLL_C 114
>>> diff --git a/include/dt-bindings/clock/tegra210-car.h b/include/dt-bindings/clock/tegra210-car.h
>>> index 44f60623f99b..a3d8d3e75728 100644
>>> --- a/include/dt-bindings/clock/tegra210-car.h
>>> +++ b/include/dt-bindings/clock/tegra210-car.h
>>> @@ -304,10 +304,10 @@
>>> #define TEGRA210_CLK_AUDIO3 274
>>> #define TEGRA210_CLK_AUDIO4 275
>>> #define TEGRA210_CLK_SPDIF 276
>>> -#define TEGRA210_CLK_CLK_OUT_1 277
>>> -#define TEGRA210_CLK_CLK_OUT_2 278
>>> -#define TEGRA210_CLK_CLK_OUT_3 279
>>> -#define TEGRA210_CLK_BLINK 280
>>> +/* 277 */
>>> +/* 278 */
>>> +/* 279 */
>>> +/* 280 */
>>> #define TEGRA210_CLK_SOR0_LVDS 281 /* deprecated */
>>> #define TEGRA210_CLK_SOR0_OUT 281
>>> #define TEGRA210_CLK_SOR1_OUT 282
>>> @@ -386,9 +386,9 @@
>>> #define TEGRA210_CLK_AUDIO3_MUX 353
>>> #define TEGRA210_CLK_AUDIO4_MUX 354
>>> #define TEGRA210_CLK_SPDIF_MUX 355
>>> -#define TEGRA210_CLK_CLK_OUT_1_MUX 356
>>> -#define TEGRA210_CLK_CLK_OUT_2_MUX 357
>>> -#define TEGRA210_CLK_CLK_OUT_3_MUX 358
>>> +/* 356 */
>>> +/* 357 */
>>> +/* 358 */
>>> #define TEGRA210_CLK_DSIA_MUX 359
>>> #define TEGRA210_CLK_DSIB_MUX 360
>>> /* 361 */
>>> diff --git a/include/dt-bindings/clock/tegra30-car.h b/include/dt-bindings/clock/tegra30-car.h
>>> index 3c90f1535551..20ef2462d9e1 100644
>>> --- a/include/dt-bindings/clock/tegra30-car.h
>>> +++ b/include/dt-bindings/clock/tegra30-car.h
>>> @@ -230,11 +230,11 @@
>>> #define TEGRA30_CLK_AUDIO3 204
>>> #define TEGRA30_CLK_AUDIO4 205
>>> #define TEGRA30_CLK_SPDIF 206
>>> -#define TEGRA30_CLK_CLK_OUT_1 207 /* (extern1) */
>>> -#define TEGRA30_CLK_CLK_OUT_2 208 /* (extern2) */
>>> -#define TEGRA30_CLK_CLK_OUT_3 209 /* (extern3) */
>>> +/* 207 */
>>> +/* 208 */
>>> +/* 209 */
>>> #define TEGRA30_CLK_SCLK 210
>>> -#define TEGRA30_CLK_BLINK 211
>>> +/* 211 */
>>> #define TEGRA30_CLK_CCLK_G 212
>>> #define TEGRA30_CLK_CCLK_LP 213
>>> #define TEGRA30_CLK_TWD 214
>>> @@ -260,9 +260,9 @@
>>> /* 297 */
>>> /* 298 */
>>> /* 299 */
>>> -#define TEGRA30_CLK_CLK_OUT_1_MUX 300
>>> -#define TEGRA30_CLK_CLK_OUT_2_MUX 301
>>> -#define TEGRA30_CLK_CLK_OUT_3_MUX 302
>>> +/* 300 */
>>> +/* 301 */
>>> +/* 302 */
>>> #define TEGRA30_CLK_AUDIO0_MUX 303
>>> #define TEGRA30_CLK_AUDIO1_MUX 304
>>> #define TEGRA30_CLK_AUDIO2_MUX 305
>>>
>>
>> This a device-tree ABI breakage and I'm not sure that it's okay to break
>> older device-trees (Pixel C Smaug board), maybe some kind of fallback is
>> needed.
>
> The Smaug support was never really official. I don't think anybody uses
> upstream on it "for real" because there's a very limited set of features
> that we do support. My understanding is that there is a community around
> Pixel C that runs their own forks with some more features and they have
> occasionally upstreamed bits and pieces of that.
>
> So given how far behind we are with Smaug, I don't think breaking ABI
> stability is really a problem in this case.
>
> Jon, we used to have a Smaug system in our internal upstream testing,
> but that's no longer the case. If we ever were to reinstate that
> testing, updating the DTB shouldn't be a problem, right? My recollection
> is that updating the DTB was always done hand in hand with the kernel
> update.

IIRC, T210 doesn't have support for SoC audio driver yet in upstream
kernel and thus Smaug only defined the audio codec in the device-tree,
which never got utilized.

2019-11-28 13:14:41

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH v2 00/11] Move PMC clocks into Tegra PMC driver

28.11.2019 15:26, Thierry Reding пишет:
> On Wed, Nov 27, 2019 at 05:31:34PM +0300, Dmitry Osipenko wrote:
>> 27.11.2019 07:59, Sowjanya Komatineni пишет:
>>> Tegra PMC has clk_out_1, clk_out_2, clk_out_3 and blink controls which
>>> are currently registered by Tegra clock driver using clk_regiser_mux and
>>> clk_register_gate which performs direct Tegra PMC register access.
>>>
>>> When Tegra PMC is in secure mode, any access from non-secure world will
>>> not go through.
>>>
>>> This patch series adds these Tegra PMC clocks and blink controls to Tegra
>>> PMC driver with PMC as clock provider and removed them from Tegra clock
>>> driver. This also adds PMC specific clock id's to use in device tree and
>>> removed clock ids of PMC clock from Tegra clock driver.
>>>
>>> This series also includes patch to update clock provider from tegra_car
>>> to pmc in the device tree tegra210-smaug.dts that uses clk_out_2 from PMC.
>>>
>>> [v2]: Changes between v1 and v2 are
>>> - v2 includes patches for adding clk_out_1, clk_out_2, clk_out_3,
>>> blink controls to Tegra PMC driver and removing clk-tegra-pmc.
>>> - feedback related to pmc clocks in Tegra PMC driver from v1
>>> - Removed patches for WB0 PLLM overrides and PLLE IDDQ PMC programming
>>> by the clock driver using helper functions from Tegra PMC.
>>>
>>> Note:
>>> To use helper functions from PMC driver, PMC early init need to
>>> happen prior to using helper functions and these helper functions are
>>> for PLLM Override and PLLE IDDQ programming in PMC during PLLM/PLLE
>>> clock registration which happen in clock_init prior to Tegra PMC
>>> probe.
>>> Moving PLLM/PLLE clocks registration to happen after Tegra PMC
>>> impacts other clocks EMC, MC and corresponding tegra_emc_init and
>>> tegra_mc_init.
>>> This implementation of configuring PMC registers thru helper
>>> functions in clock driver needs proper changes across PMC, Clock,
>>> EMC and MC inits to have it work across all Tegra platforms.
>>>
>>> Currently PLLM Override is not enabled in the bootloader so proper
>>> patches for this fix will be taken care separately.
>>
>> Hello Sowjanya,
>>
>> Could you please clarify what do you mean by "PLLM Override not enabled
>> in bootloader"?
>>
>> There is T124 Nyan Big Chromebook which is supported in upstream kernel,
>> it has PLLM Override set by bootloader. I also have T30 Nexus 7 tablet
>> which has the PLLM Override set by bootloader as well. It's not clear to
>> me whether this patch series is supposed to break these devices. If the
>> breakage is the case here, then I'm afraid you can't postpone supporting
>> the PLLM Override and a full-featured implementation is needed.
>
> For some more background on why we chose to take this shortcut for now:
> Sowjanya was looking at the full-featured implementation and that ended
> up being a can of worms. The problem is that there are various inter-
> dependencies between the PLLM override and the MC/EMC clocks.
>
> Unfortunately we depend a lot on the explicit ordering of driver probe,
> especially during early boot, so this started to get very complicated,
> very quickly.
>
> The bottom line was basically that we would need to move a whole bunch
> of clocks to register at a very late point in time and support deferred
> probe throughout in order to make it all work together nicely. Sowjanya
> had a crack at that, and while the system ended up booting, there were a
> number of errors from the MC and IOMMU drivers.
>
> At the end, we decided to take a look at that separately because, as was
> mentioned earlier, the PLLM override is not used on platforms where the
> PMC is locked down, so the existing PLLM override code is going to
> continue to work fine on the platforms where it's currently used.
>
> Thierry

Thank you and Sowjanya for the clarification.

[snip]

2019-12-02 17:12:12

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH v2 00/11] Move PMC clocks into Tegra PMC driver

28.11.2019 16:06, Dmitry Osipenko пишет:
> 28.11.2019 00:38, Sowjanya Komatineni пишет:
>>
>> On 11/27/19 9:02 AM, Sowjanya Komatineni wrote:
>>>
>>> On 11/27/19 6:31 AM, Dmitry Osipenko wrote:
>>>> 27.11.2019 07:59, Sowjanya Komatineni пишет:
>>>>> Tegra PMC has clk_out_1, clk_out_2, clk_out_3 and blink controls which
>>>>> are currently registered by Tegra clock driver using clk_regiser_mux
>>>>> and
>>>>> clk_register_gate which performs direct Tegra PMC register access.
>>>>>
>>>>> When Tegra PMC is in secure mode, any access from non-secure world will
>>>>> not go through.
>>>>>
>>>>> This patch series adds these Tegra PMC clocks and blink controls to
>>>>> Tegra
>>>>> PMC driver with PMC as clock provider and removed them from Tegra clock
>>>>> driver. This also adds PMC specific clock id's to use in device tree
>>>>> and
>>>>> removed clock ids of PMC clock from Tegra clock driver.
>>>>>
>>>>> This series also includes patch to update clock provider from tegra_car
>>>>> to pmc in the device tree tegra210-smaug.dts that uses clk_out_2
>>>>> from PMC.
>>>>>
>>>>> [v2]:    Changes between v1 and v2 are
>>>>>     - v2 includes patches for adding clk_out_1, clk_out_2, clk_out_3,
>>>>>       blink controls to Tegra PMC driver and removing clk-tegra-pmc.
>>>>>     - feedback related to pmc clocks in Tegra PMC driver from v1
>>>>>     - Removed patches for WB0 PLLM overrides and PLLE IDDQ PMC
>>>>> programming
>>>>>       by the clock driver using helper functions from Tegra PMC.
>>>>>
>>>>>         Note:
>>>>>       To use helper functions from PMC driver, PMC early init need to
>>>>>       happen prior to using helper functions and these helper
>>>>> functions are
>>>>>       for PLLM Override and PLLE IDDQ programming in PMC during
>>>>> PLLM/PLLE
>>>>>       clock registration which happen in clock_init prior to Tegra PMC
>>>>>       probe.
>>>>>       Moving PLLM/PLLE clocks registration to happen after Tegra PMC
>>>>>       impacts other clocks EMC, MC and corresponding tegra_emc_init and
>>>>>       tegra_mc_init.
>>>>>       This implementation of configuring PMC registers thru helper
>>>>>       functions in clock driver needs proper changes across PMC, Clock,
>>>>>       EMC and MC inits to have it work across all Tegra platforms.
>>>>>
>>>>>       Currently PLLM Override is not enabled in the bootloader so
>>>>> proper
>>>>>       patches for this fix will be taken care separately.
>>>> Hello Sowjanya,
>>>>
>>>> Could you please clarify what do you mean by "PLLM Override not enabled
>>>> in bootloader"?
>>>>
>>>> There is T124 Nyan Big Chromebook which is supported in upstream kernel,
>>>> it has PLLM Override set by bootloader. I also have T30 Nexus 7 tablet
>>>> which has the PLLM Override set by bootloader as well. It's not clear to
>>>> me whether this patch series is supposed to break these devices. If the
>>>> breakage is the case here, then I'm afraid you can't postpone supporting
>>>> the PLLM Override and a full-featured implementation is needed.
>>>
>>> Hi Dmitry,
>>>
>>> Secure boot currently is enabled only on Tegra210 and Tegra210
>>> bootloader doesn't enable PLLM override.
>>>
>>> So PLLM override/PLLE IDDQ being in clock driver currently will not
>>> break on any of existing Tegra platforms.
>>>
>>>>
>>>> I briefly tried to test this series on T30 and this time it doesn't hang
>>>> on boot, but somehow WiFi MMC card detection is broken. AFAIK, the WiFi
>>>> chip uses the Blink clock source and the clock should be enabled by the
>>>> MMC core because this is how DT part looks like:
>>>>
>>>> brcm_wifi_pwrseq: wifi-pwrseq {
>>>>     compatible = "mmc-pwrseq-simple";
>>>>     clocks = <&pmc TEGRA_PMC_CLK_BLINK>;
>>>>     clock-names = "ext_clock";
>>>>     reset-gpios =  <&gpio TEGRA_GPIO(D, 3) GPIO_ACTIVE_LOW>;
>>>>     post-power-on-delay-ms = <300>;
>>>>     power-off-delay-us = <300>;
>>>> };
>>>>
>>>> BTW, I  tried this series on a T20 device which also uses the Blink
>>>> clock for WiFi card and it works. So looks like this patchset has some
>>>> problem in regards to the T30 PMC clocks implementation.
>>>>
>>>> [snip]
>>>
>>> Blink init state is set to true for both Tegra20 and Tegra30 and all
>>> go through the same blink programming sequence.
>>>
>>> Will try to add more debug messages to dump registers and will test
>>> blink through device tree on T30 and will get back...
>>>
>>>
>> define value for BLINK uses BIT macro instead of just position. Will fix
>> this in v3.
>
> Thanks, will try v3 once it will be ready.
>
> I took a look through the T20 board's schematics and seems it doesn't
> use the Blink clock for the WiFi, instead it uses 32k source directly
> from PMU. While T30 board schematics tells that 32k comes out from the
> Tegra chip.

That was wrong, both T20 and T30 are identical in regards to the clk32k
wiring. I'm not sure what's difference between T20 and T30 that made
WiFi card not to work without the blink clock on T30, maybe it's a WiFi
chip difference.

> BTW, I'm curious what's the reason for having Blink clock always-ON on
> T20/30, any insights? Looks like it's just some relic from old clk
> driver and it should be safe to drop the always-ON.
>

2019-12-02 20:27:40

by Sowjanya Komatineni

[permalink] [raw]
Subject: Re: [PATCH v2 00/11] Move PMC clocks into Tegra PMC driver


On 11/28/19 5:06 AM, Dmitry Osipenko wrote:
> 28.11.2019 00:38, Sowjanya Komatineni пишет:
>> On 11/27/19 9:02 AM, Sowjanya Komatineni wrote:
>>> On 11/27/19 6:31 AM, Dmitry Osipenko wrote:
>>>> 27.11.2019 07:59, Sowjanya Komatineni пишет:
>>>>> Tegra PMC has clk_out_1, clk_out_2, clk_out_3 and blink controls which
>>>>> are currently registered by Tegra clock driver using clk_regiser_mux
>>>>> and
>>>>> clk_register_gate which performs direct Tegra PMC register access.
>>>>>
>>>>> When Tegra PMC is in secure mode, any access from non-secure world will
>>>>> not go through.
>>>>>
>>>>> This patch series adds these Tegra PMC clocks and blink controls to
>>>>> Tegra
>>>>> PMC driver with PMC as clock provider and removed them from Tegra clock
>>>>> driver. This also adds PMC specific clock id's to use in device tree
>>>>> and
>>>>> removed clock ids of PMC clock from Tegra clock driver.
>>>>>
>>>>> This series also includes patch to update clock provider from tegra_car
>>>>> to pmc in the device tree tegra210-smaug.dts that uses clk_out_2
>>>>> from PMC.
>>>>>
>>>>> [v2]:    Changes between v1 and v2 are
>>>>>     - v2 includes patches for adding clk_out_1, clk_out_2, clk_out_3,
>>>>>       blink controls to Tegra PMC driver and removing clk-tegra-pmc.
>>>>>     - feedback related to pmc clocks in Tegra PMC driver from v1
>>>>>     - Removed patches for WB0 PLLM overrides and PLLE IDDQ PMC
>>>>> programming
>>>>>       by the clock driver using helper functions from Tegra PMC.
>>>>>
>>>>>         Note:
>>>>>       To use helper functions from PMC driver, PMC early init need to
>>>>>       happen prior to using helper functions and these helper
>>>>> functions are
>>>>>       for PLLM Override and PLLE IDDQ programming in PMC during
>>>>> PLLM/PLLE
>>>>>       clock registration which happen in clock_init prior to Tegra PMC
>>>>>       probe.
>>>>>       Moving PLLM/PLLE clocks registration to happen after Tegra PMC
>>>>>       impacts other clocks EMC, MC and corresponding tegra_emc_init and
>>>>>       tegra_mc_init.
>>>>>       This implementation of configuring PMC registers thru helper
>>>>>       functions in clock driver needs proper changes across PMC, Clock,
>>>>>       EMC and MC inits to have it work across all Tegra platforms.
>>>>>
>>>>>       Currently PLLM Override is not enabled in the bootloader so
>>>>> proper
>>>>>       patches for this fix will be taken care separately.
>>>> Hello Sowjanya,
>>>>
>>>> Could you please clarify what do you mean by "PLLM Override not enabled
>>>> in bootloader"?
>>>>
>>>> There is T124 Nyan Big Chromebook which is supported in upstream kernel,
>>>> it has PLLM Override set by bootloader. I also have T30 Nexus 7 tablet
>>>> which has the PLLM Override set by bootloader as well. It's not clear to
>>>> me whether this patch series is supposed to break these devices. If the
>>>> breakage is the case here, then I'm afraid you can't postpone supporting
>>>> the PLLM Override and a full-featured implementation is needed.
>>> Hi Dmitry,
>>>
>>> Secure boot currently is enabled only on Tegra210 and Tegra210
>>> bootloader doesn't enable PLLM override.
>>>
>>> So PLLM override/PLLE IDDQ being in clock driver currently will not
>>> break on any of existing Tegra platforms.
>>>
>>>> I briefly tried to test this series on T30 and this time it doesn't hang
>>>> on boot, but somehow WiFi MMC card detection is broken. AFAIK, the WiFi
>>>> chip uses the Blink clock source and the clock should be enabled by the
>>>> MMC core because this is how DT part looks like:
>>>>
>>>> brcm_wifi_pwrseq: wifi-pwrseq {
>>>>     compatible = "mmc-pwrseq-simple";
>>>>     clocks = <&pmc TEGRA_PMC_CLK_BLINK>;
>>>>     clock-names = "ext_clock";
>>>>     reset-gpios =  <&gpio TEGRA_GPIO(D, 3) GPIO_ACTIVE_LOW>;
>>>>     post-power-on-delay-ms = <300>;
>>>>     power-off-delay-us = <300>;
>>>> };
>>>>
>>>> BTW, I  tried this series on a T20 device which also uses the Blink
>>>> clock for WiFi card and it works. So looks like this patchset has some
>>>> problem in regards to the T30 PMC clocks implementation.
>>>>
>>>> [snip]
>>> Blink init state is set to true for both Tegra20 and Tegra30 and all
>>> go through the same blink programming sequence.
>>>
>>> Will try to add more debug messages to dump registers and will test
>>> blink through device tree on T30 and will get back...
>>>
>>>
>> define value for BLINK uses BIT macro instead of just position. Will fix
>> this in v3.
> Thanks, will try v3 once it will be ready.
>
> I took a look through the T20 board's schematics and seems it doesn't
> use the Blink clock for the WiFi, instead it uses 32k source directly
> from PMU. While T30 board schematics tells that 32k comes out from the
> Tegra chip.
>
> BTW, I'm curious what's the reason for having Blink clock always-ON on
> T20/30, any insights? Looks like it's just some relic from old clk
> driver and it should be safe to drop the always-ON.

T30 cardhu uses 32K from Tegra to WIFI but its only needed to be on
during WIFI power up sequence and not required to be on during boot.

I had it enabled as existing clock driver enables it default and dont
want to break things if it was left ON intentionally.

Peter/Thierry, Any reason 32K from Blink is enabled to be ON during
clock init for T20/T30 in clock-tegra-pmc driver?

Based on the design T30 uses this for WIFI and WIFI driver should be
handling this clock enable/disable during power up/down sequence,

so we don't have to enable it default during boot right?